Assume that O_NOFOLLOW is always defined

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.

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
>

On 04/13/2017 07:33 PM, Adhemerval Zanella wrote:
>> -  /* 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

Yes, you are right, I will fix it.

Thanks,
Florian