From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H2 autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 25806 invoked from network); 27 Feb 2023 23:53:38 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 27 Feb 2023 23:53:38 -0000 Received: (qmail 19870 invoked by uid 550); 27 Feb 2023 23:53:35 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 19835 invoked from network); 27 Feb 2023 23:53:35 -0000 Date: Mon, 27 Feb 2023 18:53:22 -0500 From: Rich Felker To: musl@lists.openwall.com Message-ID: <20230227235322.GI4163@brightrain.aerifal.cx> References: <20230227194654.61114-1-izbyshev@ispras.ru> <20230227223822.GH4163@brightrain.aerifal.cx> <334e77d7dead0bfc41ba8a9eda6fdb0b@ispras.ru> <80d29e1656fea652867bc279be33df00@ispras.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <80d29e1656fea652867bc279be33df00@ispras.ru> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [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