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=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 12325 invoked from network); 16 Feb 2021 17:53:52 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 16 Feb 2021 17:53:52 -0000 Received: (qmail 5501 invoked by uid 550); 16 Feb 2021 17:53:50 -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 5468 invoked from network); 16 Feb 2021 17:53:50 -0000 Date: Tue, 16 Feb 2021 12:53:37 -0500 From: Rich Felker To: musl@lists.openwall.com Message-ID: <20210216175337.GH11590@brightrain.aerifal.cx> References: <20210216173022.759116-1-raj.khem@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210216173022.759116-1-raj.khem@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: [musl] Re: [PATCH] handle AT_SYMLINK_NOFOLLOW On Tue, Feb 16, 2021 at 09:30:22AM -0800, Khem Raj wrote: > From: Richard Purdie > > For faccessat(), AT_SYMLINK_NOFOLLOW is a supported flag by the > Linux kernel and musl should really handle it correctly rather > than return EINVAL. Noticed from code in systemd. > > Signed-off-by: Richard Purdie > Signed-off-by: Khem Raj This patch does not work. It just makes the error go away by silently doing the wrong thing instead of reporting it. > --- > src/unistd/faccessat.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/src/unistd/faccessat.c b/src/unistd/faccessat.c > index 8e8689c1..22c30bc6 100644 > --- a/src/unistd/faccessat.c > +++ b/src/unistd/faccessat.c > @@ -9,6 +9,7 @@ struct ctx { > const char *filename; > int amode; > int p; > + int flag; > }; > > static int checker(void *p) > @@ -18,7 +19,7 @@ static int checker(void *p) > if (__syscall(SYS_setregid, __syscall(SYS_getegid), -1) > || __syscall(SYS_setreuid, __syscall(SYS_geteuid), -1)) > __syscall(SYS_exit, 1); > - ret = __syscall(SYS_faccessat, c->fd, c->filename, c->amode, 0); > + ret = __syscall(SYS_faccessat, c->fd, c->filename, c->amode, c->flag & AT_SYMLINK_NOFOLLOW); The SYS_faccessat syscall does not take a flags argument. That's the whole reason for having this emulation mechanism. The 0 being left there is a historical error and it should be removed; the kernel does not inspect it and is not intended to sincw the old syscall has only 3 arguments. > __syscall(SYS_write, c->p, &ret, sizeof ret); > return 0; > } > @@ -30,11 +31,11 @@ int faccessat(int fd, const char *filename, int amode, int flag) > if (ret != -ENOSYS) return __syscall_ret(ret); > } > > - if (flag & ~AT_EACCESS) > + if (flag & ~(AT_EACCESS | AT_SYMLINK_NOFOLLOW)) > return __syscall_ret(-EINVAL); EINVAL is the normal error code Linux returns for flags not recognized/supported by the running kernel. It's also the POSIX-documented "may fail" code for this. The code *before* this test, using the new SYS_faccessat2 syscall, handles the AT_SYMLINK_NOFOLLOW flag if you have a kernel that can support it. I suppose it might be possible to emulate AT_SYMLINK_NOFOLLOW on old kernels using procfs magic symlinks, but I haven't checked the details to be sure, and IMO it does not make sense to make the fallback code here more complex when it's for a nonstandard feature that's not expected to be present on old kernels, rather than a POSIX-mandated feature like AT_EACCESS. > - if (!flag || (getuid()==geteuid() && getgid()==getegid())) > - return syscall(SYS_faccessat, fd, filename, amode); > + if (!(flag & AT_EACCESS) || (getuid()==geteuid() && getgid()==getegid())) > + return syscall(SYS_faccessat, fd, filename, amode, flag); Same issue here -- there is no flag argument. Rich