This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Assume that O_NOFOLLOW is always defined
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Thu, 13 Apr 2017 14:33:45 -0300
- Subject: Re: [PATCH] Assume that O_NOFOLLOW is always defined
- Authentication-results: sourceware.org; auth=none
- References: <20170413141642.87B5E401B4727@oldenburg.str.redhat.com>
On 13/04/2017 11:16, Florian Weimer wrote:
> 2017-04-13 Florian Weimer <fweimer@redhat.com>
>
> * csu/check_fds.c (__libc_check_standard_fds): Assume O_NOFOLLOW
> is defined.
> * elf/rtld.c (process_envvars): Likewise.
> * sysdeps/posix/shm_open.c (shm_open): Likewise.
> * elf/dl-profile.c (EXTRA_FLAGS): Remove definition. Use
> O_NOFOLLOW directly.
> * gmon/gmon.c (O_NOFOLLOW): Remove definition.
LGTM with some nits.
>
> diff --git a/csu/check_fds.c b/csu/check_fds.c
> index bec2a53..4b4eb81 100644
> --- a/csu/check_fds.c
> +++ b/csu/check_fds.c
> @@ -87,14 +87,10 @@ check_one_fd (int fd, int mode)
> void
> __libc_check_standard_fds (void)
> {
> - /* This is really paranoid but some people actually are. If /dev/null
> - should happen to be a symlink to somewhere else and not the device
> - commonly known as "/dev/null" we bail out. We can detect this with
> - the O_NOFOLLOW flag for open() but only on some system. */
> -#ifndef O_NOFOLLOW
> -# define O_NOFOLLOW 0
> -#endif
> - /* Check all three standard file descriptors. */
> + /* Check all three standard file descriptors. The O_NOFOLLOW flag
> + really paranoid but some people actually are. If /dev/null
This sentence sounds strange, shouldn't be 'The O_NOFOLLOW flag *is*
really paranoid, but some people actually are.'?
> + should happen to be a symlink to somewhere else and not the
> + device commonly known as "/dev/null" we bail out. */
> check_one_fd (STDIN_FILENO, O_WRONLY | O_NOFOLLOW);
> check_one_fd (STDOUT_FILENO, O_RDONLY | O_NOFOLLOW);
> check_one_fd (STDERR_FILENO, O_RDONLY | O_NOFOLLOW);
> diff --git a/elf/dl-profile.c b/elf/dl-profile.c
> index 01aaf31..a4f1108 100644
> --- a/elf/dl-profile.c
> +++ b/elf/dl-profile.c
> @@ -325,12 +325,7 @@ _dl_start_profile (void)
> *cp++ = '/';
> __stpcpy (__stpcpy (cp, GLRO(dl_profile)), ".profile");
>
> -#ifdef O_NOFOLLOW
> -# define EXTRA_FLAGS | O_NOFOLLOW
> -#else
> -# define EXTRA_FLAGS
> -#endif
> - fd = __open (filename, O_RDWR | O_CREAT EXTRA_FLAGS, DEFFILEMODE);
> + fd = __open (filename, O_RDWR | O_CREAT | O_NOFOLLOW, DEFFILEMODE);
> if (fd == -1)
> {
> char buf[400];
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 5986eaf..319ef06 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -2525,11 +2525,7 @@ process_envvars (enum mode *modep)
> messages to this file. */
> else if (any_debug && debug_output != NULL)
> {
> -#ifdef O_NOFOLLOW
> const int flags = O_WRONLY | O_APPEND | O_CREAT | O_NOFOLLOW;
> -#else
> - const int flags = O_WRONLY | O_APPEND | O_CREAT;
> -#endif
> size_t name_len = strlen (debug_output);
> char buf[name_len + 12];
> char *startp;
> diff --git a/gmon/gmon.c b/gmon/gmon.c
> index e9988c0..f394a78 100644
> --- a/gmon/gmon.c
> +++ b/gmon/gmon.c
> @@ -336,10 +336,6 @@ write_gmon (void)
> int fd = -1;
> char *env;
>
> -#ifndef O_NOFOLLOW
> -# define O_NOFOLLOW 0
> -#endif
> -
> env = getenv ("GMON_OUT_PREFIX");
> if (env != NULL && !__libc_enable_secure)
> {
> diff --git a/sysdeps/posix/shm_open.c b/sysdeps/posix/shm_open.c
> index aac0da4..56a9965 100644
> --- a/sysdeps/posix/shm_open.c
> +++ b/sysdeps/posix/shm_open.c
> @@ -34,9 +34,7 @@ shm_open (const char *name, int oflag, mode_t mode)
> {
> SHM_GET_NAME (EINVAL, -1, "");
>
> -# ifdef O_NOFOLLOW
> oflag |= O_NOFOLLOW;
> -# endif
> # ifdef O_CLOEXEC
> oflag |= O_CLOEXEC;
> # endif
>