* [musl] [PATCH] accept4: don't fall back to accept if we got unknown flags @ 2023-02-27 19:46 Alexey Izbyshev 2023-02-27 22:38 ` Rich Felker 0 siblings, 1 reply; 9+ messages in thread From: Alexey Izbyshev @ 2023-02-27 19:46 UTC (permalink / raw) To: musl accept4 emulation via accept ignores unknown flags, so it can spuriously succeed instead of failing (or succeed without doing the action implied by an unknown flag if it's added in a future kernel). Worse, unknown flags trigger the fallback code even on modern kernels if the real accept4 syscall returns EINVAL, because this is indistinguishable from socketcall returning EINVAL due to lack of accept4 support. Fix this by always propagating the syscall attempt failure if unknown flags are present. The behavior is still not ideal on old kernels lacking accept4 on arches with socketcall, where failing with ENOSYS instead of EINVAL returned by socketcall would be preferable, but at least modern kernels are now fine. --- src/network/accept4.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/network/accept4.c b/src/network/accept4.c index 59ab1726..bbb79507 100644 --- a/src/network/accept4.c +++ b/src/network/accept4.c @@ -9,6 +9,7 @@ int accept4(int fd, struct sockaddr *restrict addr, socklen_t *restrict len, int if (!flg) return accept(fd, addr, len); int ret = socketcall_cp(accept4, fd, addr, len, flg, 0, 0); if (ret>=0 || (errno != ENOSYS && errno != EINVAL)) return ret; + if (flg & ~(SOCK_CLOEXEC|SOCK_NONBLOCK)) return ret; ret = accept(fd, addr, len); if (ret<0) return ret; if (flg & SOCK_CLOEXEC) -- 2.39.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] accept4: don't fall back to accept if we got unknown flags 2023-02-27 19:46 [musl] [PATCH] accept4: don't fall back to accept if we got unknown flags Alexey Izbyshev @ 2023-02-27 22:38 ` Rich Felker 2023-02-27 23:42 ` Alexey Izbyshev 0 siblings, 1 reply; 9+ messages in thread From: Rich Felker @ 2023-02-27 22:38 UTC (permalink / raw) To: musl On Mon, Feb 27, 2023 at 10:46:54PM +0300, Alexey Izbyshev wrote: > accept4 emulation via accept ignores unknown flags, so it can spuriously > succeed instead of failing (or succeed without doing the action implied > by an unknown flag if it's added in a future kernel). Worse, unknown > flags trigger the fallback code even on modern kernels if the real > accept4 syscall returns EINVAL, because this is indistinguishable from > socketcall returning EINVAL due to lack of accept4 support. Fix this by > always propagating the syscall attempt failure if unknown flags are > present. > > The behavior is still not ideal on old kernels lacking accept4 on arches > with socketcall, where failing with ENOSYS instead of EINVAL returned by > socketcall would be preferable, but at least modern kernels are now > fine. Can you clarify what you mean about ENOSYS vs EINVAL here? I'm not following. Rich ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] accept4: don't fall back to accept if we got unknown flags 2023-02-27 22:38 ` Rich Felker @ 2023-02-27 23:42 ` Alexey Izbyshev 2023-02-27 23:51 ` Alexey Izbyshev 2023-02-28 17:21 ` Rich Felker 0 siblings, 2 replies; 9+ messages in thread From: Alexey Izbyshev @ 2023-02-27 23:42 UTC (permalink / raw) To: musl On 2023-02-28 01:38, Rich Felker wrote: > On Mon, Feb 27, 2023 at 10:46:54PM +0300, Alexey Izbyshev wrote: >> accept4 emulation via accept ignores unknown flags, so it can >> spuriously >> succeed instead of failing (or succeed without doing the action >> implied >> by an unknown flag if it's added in a future kernel). Worse, unknown >> flags trigger the fallback code even on modern kernels if the real >> accept4 syscall returns EINVAL, because this is indistinguishable from >> socketcall returning EINVAL due to lack of accept4 support. Fix this >> by >> always propagating the syscall attempt failure if unknown flags are >> present. >> >> The behavior is still not ideal on old kernels lacking accept4 on >> arches >> with socketcall, where failing with ENOSYS instead of EINVAL returned >> by >> socketcall would be preferable, but at least modern kernels are now >> fine. > > Can you clarify what you mean about ENOSYS vs EINVAL here? I'm not > following. > Sorry for confusion, I meant the following. On arches with socketcall, if a program running on an old kernel that doesn't support accept4 in any form calls accept4 with unknown flags, musl's accept4 will fail with EINVAL after this patch. But the reason of failure remains unclear to the programmer: is it because some flag is not supported or because accept4 is not supported at all? So I thought it'd be better to fail with ENOSYS in this case instead, although I don't know a good way to do that: the EINVAL ambiguity exists at socketcall level too, so testing whether the kernel's socketcall supports __SC_accept4 or not would probably involve calling it with known-good arguments on a separately created socket, and I certainly don't propose to do that. On the other hand, it could be argued that a function that can emulate a certain baseline feature set of another function shouldn't fail with ENOSYS at all because the real function would never do that. The two cleanest options for possibly-not-supported functions seem to be either always failing with ENOSYS if the kernel doesn't support the syscall or failing with a reasonable error if the caller requests something unsupported by the emulation. And I think accept4 satisfies the latter with this patch. As an aside, note that dup3 and pipe2 currently also ignore unknown flags on old kernels, and for pipe2 there is a valid flag (O_DIRECT) that could be silently ignored because of that. But there is no issue on newer kernels supporting the syscalls, unlike for accept4. Alexey ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] accept4: don't fall back to accept if we got unknown flags 2023-02-27 23:42 ` Alexey Izbyshev @ 2023-02-27 23:51 ` Alexey Izbyshev 2023-02-27 23:53 ` Rich Felker 2023-02-28 17:21 ` Rich Felker 1 sibling, 1 reply; 9+ messages in thread From: Alexey Izbyshev @ 2023-02-27 23:51 UTC (permalink / raw) To: musl On 2023-02-28 02:42, Alexey Izbyshev wrote: > On 2023-02-28 01:38, Rich Felker wrote: >> On Mon, Feb 27, 2023 at 10:46:54PM +0300, Alexey Izbyshev wrote: >>> accept4 emulation via accept ignores unknown flags, so it can >>> spuriously >>> succeed instead of failing (or succeed without doing the action >>> implied >>> by an unknown flag if it's added in a future kernel). Worse, unknown >>> flags trigger the fallback code even on modern kernels if the real >>> accept4 syscall returns EINVAL, because this is indistinguishable >>> from >>> socketcall returning EINVAL due to lack of accept4 support. Fix this >>> by >>> always propagating the syscall attempt failure if unknown flags are >>> present. >>> >>> The behavior is still not ideal on old kernels lacking accept4 on >>> arches >>> with socketcall, where failing with ENOSYS instead of EINVAL returned >>> by >>> socketcall would be preferable, but at least modern kernels are now >>> fine. >> >> Can you clarify what you mean about ENOSYS vs EINVAL here? I'm not >> following. >> > Sorry for confusion, I meant the following. On arches with socketcall, > if a program running on an old kernel that doesn't support accept4 in > any form calls accept4 with unknown flags, musl's accept4 will fail > with EINVAL after this patch. But the reason of failure remains > unclear to the programmer: is it because some flag is not supported or > because accept4 is not supported at all? So I thought it'd be better > to fail with ENOSYS in this case instead, although I don't know a good > way to do that: the EINVAL ambiguity exists at socketcall level too, > so testing whether the kernel's socketcall supports __SC_accept4 or > not would probably involve calling it with known-good arguments on a > separately created socket, and I certainly don't propose to do that. > > On the other hand, it could be argued that a function that can emulate > a certain baseline feature set of another function shouldn't fail with > ENOSYS at all because the real function would never do that. The two > cleanest options for possibly-not-supported functions seem to be > either always failing with ENOSYS if the kernel doesn't support the > syscall or failing with a reasonable error if the caller requests > something unsupported by the emulation. And I think accept4 satisfies > the latter with this patch. But actually it doesn't. The error should always be EINVAL in case of unknown flags for that, but the patch propagates ENOSYS on arches without socketcall. Alexey ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] accept4: don't fall back to accept if we got unknown flags 2023-02-27 23:51 ` Alexey Izbyshev @ 2023-02-27 23:53 ` Rich Felker 0 siblings, 0 replies; 9+ messages in thread From: Rich Felker @ 2023-02-27 23:53 UTC (permalink / raw) To: musl On Tue, Feb 28, 2023 at 02:51:42AM +0300, Alexey Izbyshev wrote: > On 2023-02-28 02:42, Alexey Izbyshev wrote: > >On 2023-02-28 01:38, Rich Felker wrote: > >>On Mon, Feb 27, 2023 at 10:46:54PM +0300, Alexey Izbyshev wrote: > >>>accept4 emulation via accept ignores unknown flags, so it can > >>>spuriously > >>>succeed instead of failing (or succeed without doing the > >>>action implied > >>>by an unknown flag if it's added in a future kernel). Worse, unknown > >>>flags trigger the fallback code even on modern kernels if the real > >>>accept4 syscall returns EINVAL, because this is > >>>indistinguishable from > >>>socketcall returning EINVAL due to lack of accept4 support. > >>>Fix this by > >>>always propagating the syscall attempt failure if unknown flags are > >>>present. > >>> > >>>The behavior is still not ideal on old kernels lacking accept4 > >>>on arches > >>>with socketcall, where failing with ENOSYS instead of EINVAL > >>>returned by > >>>socketcall would be preferable, but at least modern kernels are now > >>>fine. > >> > >>Can you clarify what you mean about ENOSYS vs EINVAL here? I'm not > >>following. > >> > >Sorry for confusion, I meant the following. On arches with socketcall, > >if a program running on an old kernel that doesn't support accept4 in > >any form calls accept4 with unknown flags, musl's accept4 will fail > >with EINVAL after this patch. But the reason of failure remains > >unclear to the programmer: is it because some flag is not supported or > >because accept4 is not supported at all? So I thought it'd be better > >to fail with ENOSYS in this case instead, although I don't know a good > >way to do that: the EINVAL ambiguity exists at socketcall level too, > >so testing whether the kernel's socketcall supports __SC_accept4 or > >not would probably involve calling it with known-good arguments on a > >separately created socket, and I certainly don't propose to do that. > > > >On the other hand, it could be argued that a function that can emulate > >a certain baseline feature set of another function shouldn't fail with > >ENOSYS at all because the real function would never do that. The two > >cleanest options for possibly-not-supported functions seem to be > >either always failing with ENOSYS if the kernel doesn't support the > >syscall or failing with a reasonable error if the caller requests > >something unsupported by the emulation. And I think accept4 satisfies > >the latter with this patch. > > But actually it doesn't. The error should always be EINVAL in case > of unknown flags for that, but the patch propagates ENOSYS on arches > without socketcall. Yeah, I was just about to say that. I think that's wrong. The patch should be something like: + if (flg & ~(SOCK_CLOEXEC|SOCK_NONBLOCK)) return __syscall_ret(-EINVAL); or explicitly assigning to errno. Rich ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] accept4: don't fall back to accept if we got unknown flags 2023-02-27 23:42 ` Alexey Izbyshev 2023-02-27 23:51 ` Alexey Izbyshev @ 2023-02-28 17:21 ` Rich Felker 2023-02-28 17:25 ` Rich Felker 1 sibling, 1 reply; 9+ messages in thread From: Rich Felker @ 2023-02-28 17:21 UTC (permalink / raw) To: musl On Tue, Feb 28, 2023 at 02:42:39AM +0300, Alexey Izbyshev wrote: > On 2023-02-28 01:38, Rich Felker wrote: > >On Mon, Feb 27, 2023 at 10:46:54PM +0300, Alexey Izbyshev wrote: > >>accept4 emulation via accept ignores unknown flags, so it can > >>spuriously > >>succeed instead of failing (or succeed without doing the action > >>implied > >>by an unknown flag if it's added in a future kernel). Worse, unknown > >>flags trigger the fallback code even on modern kernels if the real > >>accept4 syscall returns EINVAL, because this is indistinguishable from > >>socketcall returning EINVAL due to lack of accept4 support. Fix > >>this by > >>always propagating the syscall attempt failure if unknown flags are > >>present. > >> > >>The behavior is still not ideal on old kernels lacking accept4 > >>on arches > >>with socketcall, where failing with ENOSYS instead of EINVAL > >>returned by > >>socketcall would be preferable, but at least modern kernels are now > >>fine. > > > >Can you clarify what you mean about ENOSYS vs EINVAL here? I'm not > >following. > > > Sorry for confusion, I meant the following. On arches with > socketcall, if a program running on an old kernel that doesn't > support accept4 in any form calls accept4 with unknown flags, musl's > accept4 will fail with EINVAL after this patch. But the reason of > failure remains unclear to the programmer: is it because some flag > is not supported or because accept4 is not supported at all? So I > thought it'd be better to fail with ENOSYS in this case instead, > although I don't know a good way to do that: the EINVAL ambiguity > exists at socketcall level too, so testing whether the kernel's > socketcall supports __SC_accept4 or not would probably involve > calling it with known-good arguments on a separately created socket, > and I certainly don't propose to do that. > > On the other hand, it could be argued that a function that can > emulate a certain baseline feature set of another function shouldn't > fail with ENOSYS at all because the real function would never do > that. The two cleanest options for possibly-not-supported functions > seem to be either always failing with ENOSYS if the kernel doesn't > support the syscall or failing with a reasonable error if the caller > requests something unsupported by the emulation. And I think accept4 > satisfies the latter with this patch. > > As an aside, note that dup3 and pipe2 currently also ignore unknown > flags on old kernels, and for pipe2 there is a valid flag (O_DIRECT) > that could be silently ignored because of that. But there is no > issue on newer kernels supporting the syscalls, unlike for accept4. The dup3 situation is even worse than you thought. The dup3 syscall is only attempted if O_CLOEXEC is set in flags. If not, the rest of flags are ignored and the dup2 syscall is made. I'll make a fix. Rich ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] accept4: don't fall back to accept if we got unknown flags 2023-02-28 17:21 ` Rich Felker @ 2023-02-28 17:25 ` Rich Felker 2023-02-28 20:15 ` Alexey Izbyshev 0 siblings, 1 reply; 9+ messages in thread From: Rich Felker @ 2023-02-28 17:25 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 3050 bytes --] On Tue, Feb 28, 2023 at 12:21:01PM -0500, Rich Felker wrote: > On Tue, Feb 28, 2023 at 02:42:39AM +0300, Alexey Izbyshev wrote: > > On 2023-02-28 01:38, Rich Felker wrote: > > >On Mon, Feb 27, 2023 at 10:46:54PM +0300, Alexey Izbyshev wrote: > > >>accept4 emulation via accept ignores unknown flags, so it can > > >>spuriously > > >>succeed instead of failing (or succeed without doing the action > > >>implied > > >>by an unknown flag if it's added in a future kernel). Worse, unknown > > >>flags trigger the fallback code even on modern kernels if the real > > >>accept4 syscall returns EINVAL, because this is indistinguishable from > > >>socketcall returning EINVAL due to lack of accept4 support. Fix > > >>this by > > >>always propagating the syscall attempt failure if unknown flags are > > >>present. > > >> > > >>The behavior is still not ideal on old kernels lacking accept4 > > >>on arches > > >>with socketcall, where failing with ENOSYS instead of EINVAL > > >>returned by > > >>socketcall would be preferable, but at least modern kernels are now > > >>fine. > > > > > >Can you clarify what you mean about ENOSYS vs EINVAL here? I'm not > > >following. > > > > > Sorry for confusion, I meant the following. On arches with > > socketcall, if a program running on an old kernel that doesn't > > support accept4 in any form calls accept4 with unknown flags, musl's > > accept4 will fail with EINVAL after this patch. But the reason of > > failure remains unclear to the programmer: is it because some flag > > is not supported or because accept4 is not supported at all? So I > > thought it'd be better to fail with ENOSYS in this case instead, > > although I don't know a good way to do that: the EINVAL ambiguity > > exists at socketcall level too, so testing whether the kernel's > > socketcall supports __SC_accept4 or not would probably involve > > calling it with known-good arguments on a separately created socket, > > and I certainly don't propose to do that. > > > > On the other hand, it could be argued that a function that can > > emulate a certain baseline feature set of another function shouldn't > > fail with ENOSYS at all because the real function would never do > > that. The two cleanest options for possibly-not-supported functions > > seem to be either always failing with ENOSYS if the kernel doesn't > > support the syscall or failing with a reasonable error if the caller > > requests something unsupported by the emulation. And I think accept4 > > satisfies the latter with this patch. > > > > As an aside, note that dup3 and pipe2 currently also ignore unknown > > flags on old kernels, and for pipe2 there is a valid flag (O_DIRECT) > > that could be silently ignored because of that. But there is no > > issue on newer kernels supporting the syscalls, unlike for accept4. > > The dup3 situation is even worse than you thought. The dup3 syscall is > only attempted if O_CLOEXEC is set in flags. If not, the rest of flags > are ignored and the dup2 syscall is made. I'll make a fix. These should fix both.. [-- Attachment #2: 0001-fix-pipe2-silently-ignoring-unknown-flags-on-old-ker.patch --] [-- Type: text/plain, Size: 965 bytes --] From fb7fb5e4bd7ccb8efa691364404efc7804fad90c Mon Sep 17 00:00:00 2001 From: Rich Felker <dalias@aerifal.cx> Date: Tue, 28 Feb 2023 12:18:43 -0500 Subject: [PATCH 1/2] fix pipe2 silently ignoring unknown flags on old kernels kernels using the fallback have an inherent close-on-exec race condition and as such support for them is only best-effort anyway. however, ignoring potential new flags is still very bad behavior. instead, fail with EINVAL. --- src/unistd/pipe2.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/unistd/pipe2.c b/src/unistd/pipe2.c index f24f74fb..a096990b 100644 --- a/src/unistd/pipe2.c +++ b/src/unistd/pipe2.c @@ -8,6 +8,7 @@ int pipe2(int fd[2], int flag) if (!flag) return pipe(fd); int ret = __syscall(SYS_pipe2, fd, flag); if (ret != -ENOSYS) return __syscall_ret(ret); + if (flag & ~(O_CLOEXEC|O_NONBLOCK)) return __syscall_ret(-EINVAL); ret = pipe(fd); if (ret) return ret; if (flag & O_CLOEXEC) { -- 2.21.0 [-- Attachment #3: 0002-fix-dup3-ignoring-all-flags-but-O_CLOEXEC-on-archs-w.patch --] [-- Type: text/plain, Size: 1380 bytes --] From c99b7daafdbf1e2415bf408e67ca7813e7ddeedf Mon Sep 17 00:00:00 2001 From: Rich Felker <dalias@aerifal.cx> Date: Tue, 28 Feb 2023 12:21:23 -0500 Subject: [PATCH 2/2] fix dup3 ignoring all flags but O_CLOEXEC on archs with SYS_dup2 syscall our dup3 code wrongly skipped directly to making the SYS_dup2 syscall whenever the O_CLOEXEC bit of flags was not set. this is incorrect if any new flags are ever added, as it would silently ignore them rather than failing with an error. archs which lack SYS_dup2 were unaffected. adjust the logic so that SYS_dup3 is attempted whenever flags is nonzero, and explicitly fail with EINVAL if SYS_dup3 is unavailable and there are any unknown flags. --- src/unistd/dup3.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/unistd/dup3.c b/src/unistd/dup3.c index f919f791..1e7dc3e7 100644 --- a/src/unistd/dup3.c +++ b/src/unistd/dup3.c @@ -9,9 +9,10 @@ int __dup3(int old, int new, int flags) int r; #ifdef SYS_dup2 if (old==new) return __syscall_ret(-EINVAL); - if (flags & O_CLOEXEC) { + if (flags) { while ((r=__syscall(SYS_dup3, old, new, flags))==-EBUSY); if (r!=-ENOSYS) return __syscall_ret(r); + if (flags & ~O_CLOEXEC) return __syscall_ret(-EINVAL); } while ((r=__syscall(SYS_dup2, old, new))==-EBUSY); if (flags & O_CLOEXEC) __syscall(SYS_fcntl, new, F_SETFD, FD_CLOEXEC); -- 2.21.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] accept4: don't fall back to accept if we got unknown flags 2023-02-28 17:25 ` Rich Felker @ 2023-02-28 20:15 ` Alexey Izbyshev 2023-02-28 20:51 ` Rich Felker 0 siblings, 1 reply; 9+ messages in thread From: Alexey Izbyshev @ 2023-02-28 20:15 UTC (permalink / raw) To: musl On 2023-02-28 20:25, Rich Felker wrote: > On Tue, Feb 28, 2023 at 12:21:01PM -0500, Rich Felker wrote: >> On Tue, Feb 28, 2023 at 02:42:39AM +0300, Alexey Izbyshev wrote: >> > On 2023-02-28 01:38, Rich Felker wrote: >> > >On Mon, Feb 27, 2023 at 10:46:54PM +0300, Alexey Izbyshev wrote: >> > >>accept4 emulation via accept ignores unknown flags, so it can >> > >>spuriously >> > >>succeed instead of failing (or succeed without doing the action >> > >>implied >> > >>by an unknown flag if it's added in a future kernel). Worse, unknown >> > >>flags trigger the fallback code even on modern kernels if the real >> > >>accept4 syscall returns EINVAL, because this is indistinguishable from >> > >>socketcall returning EINVAL due to lack of accept4 support. Fix >> > >>this by >> > >>always propagating the syscall attempt failure if unknown flags are >> > >>present. >> > >> >> > >>The behavior is still not ideal on old kernels lacking accept4 >> > >>on arches >> > >>with socketcall, where failing with ENOSYS instead of EINVAL >> > >>returned by >> > >>socketcall would be preferable, but at least modern kernels are now >> > >>fine. >> > > >> > >Can you clarify what you mean about ENOSYS vs EINVAL here? I'm not >> > >following. >> > > >> > Sorry for confusion, I meant the following. On arches with >> > socketcall, if a program running on an old kernel that doesn't >> > support accept4 in any form calls accept4 with unknown flags, musl's >> > accept4 will fail with EINVAL after this patch. But the reason of >> > failure remains unclear to the programmer: is it because some flag >> > is not supported or because accept4 is not supported at all? So I >> > thought it'd be better to fail with ENOSYS in this case instead, >> > although I don't know a good way to do that: the EINVAL ambiguity >> > exists at socketcall level too, so testing whether the kernel's >> > socketcall supports __SC_accept4 or not would probably involve >> > calling it with known-good arguments on a separately created socket, >> > and I certainly don't propose to do that. >> > >> > On the other hand, it could be argued that a function that can >> > emulate a certain baseline feature set of another function shouldn't >> > fail with ENOSYS at all because the real function would never do >> > that. The two cleanest options for possibly-not-supported functions >> > seem to be either always failing with ENOSYS if the kernel doesn't >> > support the syscall or failing with a reasonable error if the caller >> > requests something unsupported by the emulation. And I think accept4 >> > satisfies the latter with this patch. >> > >> > As an aside, note that dup3 and pipe2 currently also ignore unknown >> > flags on old kernels, and for pipe2 there is a valid flag (O_DIRECT) >> > that could be silently ignored because of that. But there is no >> > issue on newer kernels supporting the syscalls, unlike for accept4. >> >> The dup3 situation is even worse than you thought. The dup3 syscall is >> only attempted if O_CLOEXEC is set in flags. If not, the rest of flags >> are ignored and the dup2 syscall is made. I'll make a fix. > Indeed, I missed that, thanks. > These should fix both.. The patches look good to me. But looking at dup3 more closely, I've noticed another bug: fcntl is called even if SYS_dup2 fails. So on kernels where SYS_dup3 is unavailable, dup3(-1, fd, O_CLOEXEC) will wrongly try to set FD_CLOEXEC on fd. Alexey ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] [PATCH] accept4: don't fall back to accept if we got unknown flags 2023-02-28 20:15 ` Alexey Izbyshev @ 2023-02-28 20:51 ` Rich Felker 0 siblings, 0 replies; 9+ messages in thread From: Rich Felker @ 2023-02-28 20:51 UTC (permalink / raw) To: musl On Tue, Feb 28, 2023 at 11:15:06PM +0300, Alexey Izbyshev wrote: > On 2023-02-28 20:25, Rich Felker wrote: > >On Tue, Feb 28, 2023 at 12:21:01PM -0500, Rich Felker wrote: > >>On Tue, Feb 28, 2023 at 02:42:39AM +0300, Alexey Izbyshev wrote: > >>> On 2023-02-28 01:38, Rich Felker wrote: > >>> >On Mon, Feb 27, 2023 at 10:46:54PM +0300, Alexey Izbyshev wrote: > >>> >>accept4 emulation via accept ignores unknown flags, so it can > >>> >>spuriously > >>> >>succeed instead of failing (or succeed without doing the action > >>> >>implied > >>> >>by an unknown flag if it's added in a future kernel). Worse, unknown > >>> >>flags trigger the fallback code even on modern kernels if the real > >>> >>accept4 syscall returns EINVAL, because this is indistinguishable from > >>> >>socketcall returning EINVAL due to lack of accept4 support. Fix > >>> >>this by > >>> >>always propagating the syscall attempt failure if unknown flags are > >>> >>present. > >>> >> > >>> >>The behavior is still not ideal on old kernels lacking accept4 > >>> >>on arches > >>> >>with socketcall, where failing with ENOSYS instead of EINVAL > >>> >>returned by > >>> >>socketcall would be preferable, but at least modern kernels are now > >>> >>fine. > >>> > > >>> >Can you clarify what you mean about ENOSYS vs EINVAL here? I'm not > >>> >following. > >>> > > >>> Sorry for confusion, I meant the following. On arches with > >>> socketcall, if a program running on an old kernel that doesn't > >>> support accept4 in any form calls accept4 with unknown flags, musl's > >>> accept4 will fail with EINVAL after this patch. But the reason of > >>> failure remains unclear to the programmer: is it because some flag > >>> is not supported or because accept4 is not supported at all? So I > >>> thought it'd be better to fail with ENOSYS in this case instead, > >>> although I don't know a good way to do that: the EINVAL ambiguity > >>> exists at socketcall level too, so testing whether the kernel's > >>> socketcall supports __SC_accept4 or not would probably involve > >>> calling it with known-good arguments on a separately created socket, > >>> and I certainly don't propose to do that. > >>> > >>> On the other hand, it could be argued that a function that can > >>> emulate a certain baseline feature set of another function shouldn't > >>> fail with ENOSYS at all because the real function would never do > >>> that. The two cleanest options for possibly-not-supported functions > >>> seem to be either always failing with ENOSYS if the kernel doesn't > >>> support the syscall or failing with a reasonable error if the caller > >>> requests something unsupported by the emulation. And I think accept4 > >>> satisfies the latter with this patch. > >>> > >>> As an aside, note that dup3 and pipe2 currently also ignore unknown > >>> flags on old kernels, and for pipe2 there is a valid flag (O_DIRECT) > >>> that could be silently ignored because of that. But there is no > >>> issue on newer kernels supporting the syscalls, unlike for accept4. > >> > >>The dup3 situation is even worse than you thought. The dup3 syscall is > >>only attempted if O_CLOEXEC is set in flags. If not, the rest of flags > >>are ignored and the dup2 syscall is made. I'll make a fix. > > > Indeed, I missed that, thanks. > > >These should fix both.. > > The patches look good to me. > > But looking at dup3 more closely, I've noticed another bug: fcntl is > called even if SYS_dup2 fails. So on kernels where SYS_dup3 is > unavailable, dup3(-1, fd, O_CLOEXEC) will wrongly try to set > FD_CLOEXEC on fd. Thanks for catching that. I'll fix it too. Rich ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-02-28 20:52 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-02-27 19:46 [musl] [PATCH] accept4: don't fall back to accept if we got unknown flags Alexey Izbyshev 2023-02-27 22:38 ` Rich Felker 2023-02-27 23:42 ` Alexey Izbyshev 2023-02-27 23:51 ` Alexey Izbyshev 2023-02-27 23:53 ` Rich Felker 2023-02-28 17:21 ` Rich Felker 2023-02-28 17:25 ` Rich Felker 2023-02-28 20:15 ` Alexey Izbyshev 2023-02-28 20:51 ` Rich Felker
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).