Date: Mon, 27 Feb 2023 18:53:22 -0500 From: Rich Felker <dalias@...c.org> To: musl@...ts.openwall.com Subject: Re: [PATCH] accept4: don't fall back to accept if we got unknown flags 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
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.