Message ID | 20230810135411.1000205-1-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | [v2] linux: Make fdopendir fail with O_PATH (BZ 30373) | expand |
Ping, also the correct bug it fixes is 30737. On 10/08/23 10:54, Adhemerval Zanella wrote: > It is not strictly required by the POSIX, since O_PATH is a Linux > extension, but it is QoI to fail early instead of at readdir. Also > the check is free, since fdopendir already checks if the file > descriptor is opened for read. > > Checked on x86_64-linux-gnu. > --- > sysdeps/unix/sysv/linux/Makefile | 1 + > sysdeps/unix/sysv/linux/fdopendir.c | 8 +++- > .../unix/sysv/linux/tst-fdopendir-o_path.c | 48 +++++++++++++++++++ > 3 files changed, 56 insertions(+), 1 deletion(-) > create mode 100644 sysdeps/unix/sysv/linux/tst-fdopendir-o_path.c > > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile > index be801e3be4..ec0ba3eb05 100644 > --- a/sysdeps/unix/sysv/linux/Makefile > +++ b/sysdeps/unix/sysv/linux/Makefile > @@ -199,6 +199,7 @@ tests += \ > tst-clone3 \ > tst-epoll \ > tst-fanotify \ > + tst-fdopendir-o_path \ > tst-getauxval \ > tst-gettid \ > tst-gettid-kill \ > diff --git a/sysdeps/unix/sysv/linux/fdopendir.c b/sysdeps/unix/sysv/linux/fdopendir.c > index 861345396c..839690f4d3 100644 > --- a/sysdeps/unix/sysv/linux/fdopendir.c > +++ b/sysdeps/unix/sysv/linux/fdopendir.c > @@ -37,10 +37,16 @@ __fdopendir (int fd) > return NULL; > } > > - /* Make sure the descriptor allows for reading. */ > int flags = __fcntl64_nocancel (fd, F_GETFL); > if (__glibc_unlikely (flags == -1)) > return NULL; > + /* Fail early for descriptors opened with O_PATH. */ > + if (__glibc_unlikely ((flags & O_PATH) == O_PATH)) > + { > + __set_errno (EBADF); > + return NULL; > + } > + /* Make sure the descriptor allows for reading. */ > if (__glibc_unlikely ((flags & O_ACCMODE) == O_WRONLY)) > { > __set_errno (EINVAL); > diff --git a/sysdeps/unix/sysv/linux/tst-fdopendir-o_path.c b/sysdeps/unix/sysv/linux/tst-fdopendir-o_path.c > new file mode 100644 > index 0000000000..2531cf8ddb > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/tst-fdopendir-o_path.c > @@ -0,0 +1,48 @@ > +/* Check if fdopendir fails with file descriptor opened with O_PATH (BZ 30737) > + Copyright (C) 2023 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <errno.h> > +#include <fcntl.h> > +#include <dirent.h> > +#include <support/check.h> > +#include <support/temp_file.h> > +#include <support/xunistd.h> > + > +static int > +do_test (void) > +{ > + char *dirname = support_create_temp_directory ("tst-fdopendir-o_path"); > + > + { > + int fd = xopen (dirname, O_RDONLY | O_DIRECTORY, 0600); > + DIR *dir = fdopendir (fd); > + TEST_VERIFY_EXIT (dir != NULL); > + closedir (dir); > + } > + > + { > + int fd = xopen (dirname, O_RDONLY | O_PATH | O_DIRECTORY, 0600); > + TEST_VERIFY (fdopendir (fd) == NULL); > + TEST_COMPARE (errno, EBADF); > + xclose (fd); > + } > + > + return 0; > +} > + > +#include <support/test-driver.c>
On 2023-08-10 06:54, Adhemerval Zanella via Libc-alpha wrote: > + /* Fail early for descriptors opened with O_PATH. */ > + if (__glibc_unlikely ((flags & O_PATH) == O_PATH)) > + { > + __set_errno (EBADF); > + return NULL; > + } Shouldn't this be (flags & O_PATH), not ((flags & O_PATH) == O_PATH)? The latter suggests more complexity than there really is.
On 31/08/23 17:21, Paul Eggert wrote: > On 2023-08-10 06:54, Adhemerval Zanella via Libc-alpha wrote: >> + /* Fail early for descriptors opened with O_PATH. */ >> + if (__glibc_unlikely ((flags & O_PATH) == O_PATH)) >> + { >> + __set_errno (EBADF); >> + return NULL; >> + } > > Shouldn't this be (flags & O_PATH), not ((flags & O_PATH) == O_PATH)? The latter suggests more complexity than there really is. Indeed, I will send an updated version.
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index be801e3be4..ec0ba3eb05 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -199,6 +199,7 @@ tests += \ tst-clone3 \ tst-epoll \ tst-fanotify \ + tst-fdopendir-o_path \ tst-getauxval \ tst-gettid \ tst-gettid-kill \ diff --git a/sysdeps/unix/sysv/linux/fdopendir.c b/sysdeps/unix/sysv/linux/fdopendir.c index 861345396c..839690f4d3 100644 --- a/sysdeps/unix/sysv/linux/fdopendir.c +++ b/sysdeps/unix/sysv/linux/fdopendir.c @@ -37,10 +37,16 @@ __fdopendir (int fd) return NULL; } - /* Make sure the descriptor allows for reading. */ int flags = __fcntl64_nocancel (fd, F_GETFL); if (__glibc_unlikely (flags == -1)) return NULL; + /* Fail early for descriptors opened with O_PATH. */ + if (__glibc_unlikely ((flags & O_PATH) == O_PATH)) + { + __set_errno (EBADF); + return NULL; + } + /* Make sure the descriptor allows for reading. */ if (__glibc_unlikely ((flags & O_ACCMODE) == O_WRONLY)) { __set_errno (EINVAL); diff --git a/sysdeps/unix/sysv/linux/tst-fdopendir-o_path.c b/sysdeps/unix/sysv/linux/tst-fdopendir-o_path.c new file mode 100644 index 0000000000..2531cf8ddb --- /dev/null +++ b/sysdeps/unix/sysv/linux/tst-fdopendir-o_path.c @@ -0,0 +1,48 @@ +/* Check if fdopendir fails with file descriptor opened with O_PATH (BZ 30737) + Copyright (C) 2023 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <fcntl.h> +#include <dirent.h> +#include <support/check.h> +#include <support/temp_file.h> +#include <support/xunistd.h> + +static int +do_test (void) +{ + char *dirname = support_create_temp_directory ("tst-fdopendir-o_path"); + + { + int fd = xopen (dirname, O_RDONLY | O_DIRECTORY, 0600); + DIR *dir = fdopendir (fd); + TEST_VERIFY_EXIT (dir != NULL); + closedir (dir); + } + + { + int fd = xopen (dirname, O_RDONLY | O_PATH | O_DIRECTORY, 0600); + TEST_VERIFY (fdopendir (fd) == NULL); + TEST_COMPARE (errno, EBADF); + xclose (fd); + } + + return 0; +} + +#include <support/test-driver.c>