Assume that O_NOFOLLOW is always defined
Commit Message
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.
Comments
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
@@ -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
+ 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);
@@ -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];
@@ -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;
@@ -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)
{
@@ -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