Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compilation error on Loong64. #54

Closed
wants to merge 1 commit into from

Conversation

liuxiang88
Copy link

The system call statx is used instead of stat, lstat and fstat on LoongArch64.

Signed-off-by: liuxiang <liuxiang@loongson.cn>
ThomasHabets added a commit that referenced this pull request Jan 15, 2024
Alternative solution to #54
ThomasHabets added a commit that referenced this pull request Jan 15, 2024
Alternative solution to #54

Really this should be checking that the path name is empty, too,
because the first arg is `dirfd`, and so this filter is based on:

```
By file descriptor
 If pathname is an empty string and the AT_EMPTY_PATH flag  is  speci‐
 fied  in  flags (see below), then the target file is the one referred
 to by the file descriptor dirfd.
```
@ThomasHabets
Copy link
Owner

ThomasHabets commented Jan 15, 2024

Thanks for the PR.

Could you try the alternate fix I implemented in 99b5445 ?

@liuxiang88
Copy link
Author

liuxiang88 commented Jan 16, 2024 via email

@ThomasHabets
Copy link
Owner

Thanks for confirming!

@whitslack
Copy link

Same issue on ARMv6 (Raspberry Pi 1).

uname({sysname="Linux", nodename="raspberrypi", ...}) = 0
setsockopt(5, SOL_SOCKET, SO_ATTACH_FILTER, {len=1, filter=0xb6f5d1d4}, 8) = 0
fcntl64(5, F_GETFL)                     = 0x802 (flags O_RDWR|O_NONBLOCK)
fcntl64(5, F_SETFL, O_RDWR|O_NONBLOCK)  = 0
recv(5, 0xbe82ad90, 1, MSG_TRUNC)       = -1 EAGAIN (Resource temporarily unavailable)
fcntl64(5, F_SETFL, O_RDWR|O_NONBLOCK)  = 0
setsockopt(5, SOL_SOCKET, SO_ATTACH_FILTER, {len=4, filter=0xc6be80}, 8) = 0
socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 4
ioctl(4, SIOCGIFHWADDR, {ifr_name="eth0", ifr_hwaddr={sa_family=ARPHRD_ETHER, sa_data=b8:27:eb:ee:0c:5f}}) = 0
close(4)                                = 0
socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = 4
ioctl(4, SIOCGIFADDR, {ifr_name="eth0", ifr_addr={sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("192.168.1.36")}}) = 0
close(4)                                = 0
rt_sigaction(SIGINT, {sa_handler=0x44a9b8, sa_mask=[INT], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0xb6d8c610}, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
seccomp(SECCOMP_SET_MODE_STRICT, 0x1, NULL) = -1 EINVAL (Invalid argument)
seccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_TSYNC, NULL) = -1 EFAULT (Bad address)
seccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_LOG, NULL) = -1 EFAULT (Bad address)
seccomp(SECCOMP_GET_ACTION_AVAIL, 0, [SECCOMP_RET_LOG]) = 0
seccomp(SECCOMP_GET_ACTION_AVAIL, 0, [SECCOMP_RET_KILL_PROCESS]) = 0
seccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_SPEC_ALLOW, NULL) = -1 EFAULT (Bad address)
seccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_NEW_LISTENER, NULL) = -1 EFAULT (Bad address)
seccomp(SECCOMP_GET_NOTIF_SIZES, 0, {seccomp_notif=80, seccomp_notif_resp=24, seccomp_data=64}) = 0
seccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_TSYNC_ESRCH, NULL) = -1 EFAULT (Bad address)
prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)  = 0
seccomp(SECCOMP_SET_MODE_FILTER, 0, {len=19, filter=0xc7acb0}) = 0
statx(1, "", AT_STATX_SYNC_AS_STAT|AT_NO_AUTOMOUNT|AT_EMPTY_PATH, STATX_BASIC_STATS, {stx_mask=0, stx_attributes=0, ...}) = 1
+++ killed by SIGSYS +++
Bad system call

99b5445 gets it past that hurdle, but now it's dying on clock_gettime64. I think I'll just disable seccomp. 😐

@ThomasHabets
Copy link
Owner

@whitslack could you please try adding seccomp_allow(ctx, "clock_gettime64"); next to the other calls to seccomp_allow(…) calls in src/arping.c, to see if it fixes it?

But sigh, yeah seccomp really does suck. I'll continue to claim that there's no way to use it correctly, until someone proves otherwise.

@whitslack
Copy link

could you please try adding seccomp_allow(ctx, "clock_gettime64"); next to the other calls to seccomp_allow(…) calls in src/arping.c, to see if it fixes it?

Yes, that gets it working.

But sigh, yeah seccomp really does suck. I'll continue to claim that there's no way to use it correctly, until someone proves otherwise.

That is my opinion as well. I have had to contribute patches to other projects using seccomp to add missing syscalls to them as well. The principal issue is that standard library functions do not specify which syscalls they will invoke, so there is actually no conformant way to compute the set of needed syscalls for a given set of standard library function calls.

By the way, I'm not sure that your "checking seccomp syscall {fstat,statx,nonexistant}... yes" Autoconf test is a good idea, as that doesn't seem like it will work correctly when cross-compiling. (I haven't tried it.) Also, the correct spelling is "nonexistent".

@ThomasHabets
Copy link
Owner

It only tries to compile & link, not run. But yes, it does assume that the kernel will be happy with constants sent to it by a userspace that only existed on a different machine. I guess it assumes a "similar enough" kernel version, which may not be true.

@ThomasHabets
Copy link
Owner

@whitslack have you found this to be a problem with default settings? Unless you give --enable-seccomp to ./configure, it should default to off, and only crash if there's an unknown syscall if you provide -z.

I don't think I'll ever be able to default to --enable-seccomp.

ThomasHabets added a commit that referenced this pull request May 8, 2024
@whitslack
Copy link

have you found this to be a problem with default settings?

Gentoo's net-analyzer/arping ebuild has its seccomp USE flag enabled by default. That translates to passing --enable-seccomp to configure. I had to explicitly disable the flag to configure with --disable-seccomp.

it should […] only crash if there's an unknown syscall if you provide -z.

arping.8 says the default for -z “depends on compile options.” I wasn't aware of the existence of -z/-Z; it defaulted to on.

@whitslack
Copy link

@ThomasHabets: Why do you test for existence of syscalls at compile time? That can break if you run on a different kernel than you compile on. Shouldn't you unconditionally seccomp_syscall_resolve_name every possible syscall and only seccomp_rule_add the ones that actually resolve at runtime?

@ThomasHabets
Copy link
Owner

@whitslack that sounds quite reasonable. Thanks for the pointer. I'll get on that.

ThomasHabets added a commit that referenced this pull request May 15, 2024
In case the build system is too different from the target system.

Bug #54
ThomasHabets added a commit that referenced this pull request May 15, 2024
@ThomasHabets
Copy link
Owner

@whitslack that's now done. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants