ThomasHabets / arping

ARP Ping
http://www.habets.pp.se/synscan/programs.php
GNU General Public License v2.0
398 stars 62 forks source link

Fix compilation error on Loong64. #54

Closed liuxiang88 closed 6 months ago

liuxiang88 commented 6 months ago

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

ThomasHabets commented 6 months ago

Thanks for the PR.

Could you try the alternate fix I implemented in https://github.com/ThomasHabets/arping/commit/99b5445cda5da420983ce1fe4ecd550e9638d523 ?

liuxiang88 commented 6 months ago

Yes,it is worked

-----原始邮件----- 发件人:"Thomas Habets" @.> 发送时间:2024-01-15 22:19:19 (星期一) 收件人: ThomasHabets/arping @.> 抄送: liuxiang88 @.>, Author @.> 主题: Re: [ThomasHabets/arping] Fix compilation error on Loong64. (PR #54)

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

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

ThomasHabets commented 6 months ago

Thanks for confirming!

whitslack commented 2 months ago

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

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

ThomasHabets commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@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.

whitslack commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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

ThomasHabets commented 2 months ago

@whitslack that's now done. Thanks again.