[1/1] linux: open and openat ignore 'mode' with O_TMPFILE in flags

Message ID 3056bdb5c0667f1b15f5ed04ee9fafec0e10c372.1414706994.git.e@nanocritical.com
State Superseded
Headers

Commit Message

Eric Rannaud Oct. 30, 2014, 10:09 p.m. UTC
  Both open and openat load the last argument 'mode' lazily, using
va_arg() only if O_CREAT is found in oflag. This is wrong, mode is also
necessary if O_TMPFILE is in oflag.

By chance on x86_64, the problem wasn't evident when using O_TMPFILE
with open, as the 3rd argument of open, even when not loaded with
va_arg, is left untouched in the RDX, where the syscall expects it.

However, openat was not so lucky, and O_TMPFILE couldn't be used: mode
is the 4th argument, in RCX, but the syscall expects its 4th argument in
a different register than the glibc wrapper, in R10.

Introduce a system-specific macro __OPEN_NEEDS_MODE(oflag): by default,
it tests for O_CREAT in oflag; on Linux, it tests for either O_CREAT or
__O_TMPFILE (even if __USE_GNU is not defined).

Tested on Linux x86_64.

	[BZ #17523]
	* io/fcntl.h: Default definition of __OPEN_NEEDS_MODE(oflag).
	* sysdeps/unix/sysv/linux/bits/fcntl-linux.h: Linux-specific definition of __OPEN_NEEDS_MODE().
	* io/bits/fcntl2.h: Use __OPEN_NEEDS_MODE().
	* io/open.c: Likewise.
	* io/open64.c: Likewise.
	* io/open64_2.c: Likewise.
	* io/open_2.c: Likewise.
	* io/openat.c: Likewise.
	* io/openat64.c: Likewise.
	* io/openat64_2.c: Likewise.
	* io/openat_2.c: Likewise.
	* sysdeps/mach/hurd/open.c: Likewise.
	* sysdeps/mach/hurd/openat.c: Likewise.
	* sysdeps/posix/open64.c: Likewise.
	* sysdeps/unix/sysv/linux/dl-openat64.c: Likewise.
	* sysdeps/unix/sysv/linux/generic/open.c: Likewise.
	* sysdeps/unix/sysv/linux/generic/open64.c: Likewise.
	* sysdeps/unix/sysv/linux/open64.c: Likewise.
	* sysdeps/unix/sysv/linux/openat.c: Likewise.
---
 io/bits/fcntl2.h                           | 20 ++++++++++----------
 io/fcntl.h                                 |  6 +++++-
 io/open.c                                  |  4 ++--
 io/open64.c                                |  4 ++--
 io/open64_2.c                              |  4 ++--
 io/open_2.c                                |  4 ++--
 io/openat.c                                |  4 ++--
 io/openat64.c                              |  4 ++--
 io/openat64_2.c                            |  4 ++--
 io/openat_2.c                              |  4 ++--
 sysdeps/mach/hurd/open.c                   |  4 ++--
 sysdeps/mach/hurd/openat.c                 |  4 ++--
 sysdeps/posix/open64.c                     |  4 ++--
 sysdeps/unix/sysv/linux/bits/fcntl-linux.h |  3 +++
 sysdeps/unix/sysv/linux/dl-openat64.c      |  2 +-
 sysdeps/unix/sysv/linux/generic/open.c     |  6 +++---
 sysdeps/unix/sysv/linux/generic/open64.c   |  4 ++--
 sysdeps/unix/sysv/linux/open64.c           |  4 ++--
 sysdeps/unix/sysv/linux/openat.c           |  2 +-
 19 files changed, 49 insertions(+), 42 deletions(-)
  

Comments

Roland McGrath Oct. 30, 2014, 10:39 p.m. UTC | #1
> Introduce a system-specific macro __OPEN_NEEDS_MODE(oflag): by default,
> it tests for O_CREAT in oflag; on Linux, it tests for either O_CREAT or
> __O_TMPFILE (even if __USE_GNU is not defined).

This doesn't really need to be system-specific.  The values of O_*
constants are system-specific, and even what subset of all known O_*
constants are made available is system-specific, but the meaning of each
given O_FOO constant in the API (including things like whether it implies
using a mode argument for open/openat) is part of the generic glibc API.

So certainly a convenience macro is worthwhile to avoid repeating a
nontrivial expression.  But it doesn't need to be system-specific.
It can just be based on #if.

>  __errordecl (__open_missing_mode,
> -	     "open with O_CREAT in second argument needs 3 arguments");
> +	     "open with e.g. O_CREAT in second argument needs 3 arguments");

Keep these explicit about the flags required.  It's OK to say "O_CREAT or
O_TMPFILE" even if O_TMPFILE might not actually exist.  For extra credit,
you could make the checks more specific and different errors for the
O_CREAT and O_TMPFILE cases.  But there is not much benefit to that.

> +      if (__OPEN_NEEDS_MODE(__oflag) && __va_arg_pack_len () < 1)

Space before paren.  I won't cite all the other places with the same issue.

>  /* Open FILE and return a new file descriptor for it, or -1 on error.
> -   OFLAG determines the type of access used.  If O_CREAT is on OFLAG,
> +   OFLAG determines the type of access used.  If __OPEN_NEEDS_MODE(OFLAG),
>     the third argument is taken as a `mode_t', the mode of the created file.

Don't make comment changes like this.  The comments in the public header
file are there to be user documentation of the API.  So they should never
mention internal symbols or whatnot, only the aspects of the API that
someone writing a call to this function would use directly.  Again, it's
fine to say "... or O_TMPFILE" without further qualification regardless of
whether O_TMPFILE is actually available.


Thanks,
Roland
  
Eric Rannaud Oct. 30, 2014, 10:56 p.m. UTC | #2
On Thu, Oct 30, 2014 at 3:39 PM, Roland McGrath <roland@hack.frob.com> wrote:
> > Introduce a system-specific macro __OPEN_NEEDS_MODE(oflag): by default,
> > it tests for O_CREAT in oflag; on Linux, it tests for either O_CREAT or
> > __O_TMPFILE (even if __USE_GNU is not defined).
>
> This doesn't really need to be system-specific.  The values of O_*
> constants are system-specific, and even what subset of all known O_*
> constants are made available is system-specific, but the meaning of each
> given O_FOO constant in the API (including things like whether it implies
> using a mode argument for open/openat) is part of the generic glibc API.
>
> So certainly a convenience macro is worthwhile to avoid repeating a
> nontrivial expression.  But it doesn't need to be system-specific.
> It can just be based on #if.

Understood. In io/fcntl.h:

  #ifdef __O_TMPFILE
  # define __OPEN_NEEDS_MODE(oflag) \
    ( ((oflag) & O_CREAT) != 0 || ((oflag) & __O_TMPFILE) == __O_TMPFILE )
  #else
  # define __OPEN_NEEDS_MODE(oflag) ( ((oflag) & O_CREAT) != 0 )
  #endif

Do I test for ifdef __O_TMPFILE or for O_TMPFILE (only available with
__USE_GNU)? Is the glibc itself compiled with __USE_GNU?


> >  __errordecl (__open_missing_mode,
> > -          "open with O_CREAT in second argument needs 3 arguments");
> > +          "open with e.g. O_CREAT in second argument needs 3 arguments");
>
> Keep these explicit about the flags required.  It's OK to say "O_CREAT or
> O_TMPFILE" even if O_TMPFILE might not actually exist.  For extra credit,
> you could make the checks more specific and different errors for the
> O_CREAT and O_TMPFILE cases.  But there is not much benefit to that.

OK.

> > +      if (__OPEN_NEEDS_MODE(__oflag) && __va_arg_pack_len () < 1)
>
> Space before paren.  I won't cite all the other places with the same issue.

Thanks.

> >  /* Open FILE and return a new file descriptor for it, or -1 on error.
> > -   OFLAG determines the type of access used.  If O_CREAT is on OFLAG,
> > +   OFLAG determines the type of access used.  If __OPEN_NEEDS_MODE(OFLAG),
> >     the third argument is taken as a `mode_t', the mode of the created file.
>
> Don't make comment changes like this.  The comments in the public header
> file are there to be user documentation of the API.  So they should never
> mention internal symbols or whatnot, only the aspects of the API that
> someone writing a call to this function would use directly.  Again, it's
> fine to say "... or O_TMPFILE" without further qualification regardless of
> whether O_TMPFILE is actually available.

Thanks, I misunderstood the target audience for these comments.

Eric
  

Patch

diff --git a/io/bits/fcntl2.h b/io/bits/fcntl2.h
index 4f13b1070673..677e349f4ca3 100644
--- a/io/bits/fcntl2.h
+++ b/io/bits/fcntl2.h
@@ -20,8 +20,8 @@ 
 # error "Never include <bits/fcntl2.h> directly; use <fcntl.h> instead."
 #endif
 
-/* Check that calls to open and openat with O_CREAT set have an
-   appropriate third/fourth parameter.  */
+/* Check that calls to open and openat have an appropriate third/fourth
+   parameter.  */
 #ifndef __USE_FILE_OFFSET64
 extern int __open_2 (const char *__path, int __oflag) __nonnull ((1));
 extern int __REDIRECT (__open_alias, (const char *__path, int __oflag, ...),
@@ -35,7 +35,7 @@  extern int __REDIRECT (__open_alias, (const char *__path, int __oflag, ...),
 __errordecl (__open_too_many_args,
 	     "open can be called either with 2 or 3 arguments, not more");
 __errordecl (__open_missing_mode,
-	     "open with O_CREAT in second argument needs 3 arguments");
+	     "open with e.g. O_CREAT in second argument needs 3 arguments");
 
 __fortify_function int
 open (const char *__path, int __oflag, ...)
@@ -45,7 +45,7 @@  open (const char *__path, int __oflag, ...)
 
   if (__builtin_constant_p (__oflag))
     {
-      if ((__oflag & O_CREAT) != 0 && __va_arg_pack_len () < 1)
+      if (__OPEN_NEEDS_MODE(__oflag) && __va_arg_pack_len () < 1)
 	{
 	  __open_missing_mode ();
 	  return __open_2 (__path, __oflag);
@@ -67,7 +67,7 @@  extern int __REDIRECT (__open64_alias, (const char *__path, int __oflag,
 __errordecl (__open64_too_many_args,
 	     "open64 can be called either with 2 or 3 arguments, not more");
 __errordecl (__open64_missing_mode,
-	     "open64 with O_CREAT in second argument needs 3 arguments");
+	     "open64 with e.g. O_CREAT in second argument needs 3 arguments");
 
 __fortify_function int
 open64 (const char *__path, int __oflag, ...)
@@ -77,7 +77,7 @@  open64 (const char *__path, int __oflag, ...)
 
   if (__builtin_constant_p (__oflag))
     {
-      if ((__oflag & O_CREAT) != 0 && __va_arg_pack_len () < 1)
+      if (__OPEN_NEEDS_MODE(__oflag) && __va_arg_pack_len () < 1)
 	{
 	  __open64_missing_mode ();
 	  return __open64_2 (__path, __oflag);
@@ -111,7 +111,7 @@  extern int __REDIRECT (__openat_alias, (int __fd, const char *__path,
 __errordecl (__openat_too_many_args,
 	     "openat can be called either with 3 or 4 arguments, not more");
 __errordecl (__openat_missing_mode,
-	     "openat with O_CREAT in third argument needs 4 arguments");
+	     "openat with e.g. O_CREAT in third argument needs 4 arguments");
 
 __fortify_function int
 openat (int __fd, const char *__path, int __oflag, ...)
@@ -121,7 +121,7 @@  openat (int __fd, const char *__path, int __oflag, ...)
 
   if (__builtin_constant_p (__oflag))
     {
-      if ((__oflag & O_CREAT) != 0 && __va_arg_pack_len () < 1)
+      if (__OPEN_NEEDS_MODE(__oflag) && __va_arg_pack_len () < 1)
 	{
 	  __openat_missing_mode ();
 	  return __openat_2 (__fd, __path, __oflag);
@@ -145,7 +145,7 @@  extern int __REDIRECT (__openat64_alias, (int __fd, const char *__path,
 __errordecl (__openat64_too_many_args,
 	     "openat64 can be called either with 3 or 4 arguments, not more");
 __errordecl (__openat64_missing_mode,
-	     "openat64 with O_CREAT in third argument needs 4 arguments");
+	     "openat64 with e.g. O_CREAT in third argument needs 4 arguments");
 
 __fortify_function int
 openat64 (int __fd, const char *__path, int __oflag, ...)
@@ -155,7 +155,7 @@  openat64 (int __fd, const char *__path, int __oflag, ...)
 
   if (__builtin_constant_p (__oflag))
     {
-      if ((__oflag & O_CREAT) != 0 && __va_arg_pack_len () < 1)
+      if (__OPEN_NEEDS_MODE(__oflag) && __va_arg_pack_len () < 1)
 	{
 	  __openat64_missing_mode ();
 	  return __openat64_2 (__fd, __path, __oflag);
diff --git a/io/fcntl.h b/io/fcntl.h
index cf512dd27f8c..e73d701f3eb0 100644
--- a/io/fcntl.h
+++ b/io/fcntl.h
@@ -34,6 +34,10 @@  __BEGIN_DECLS
    numbers and flag bits for `open', `fcntl', et al.  */
 #include <bits/fcntl.h>
 
+#ifndef __OPEN_NEEDS_MODE
+# define __OPEN_NEEDS_MODE(oflag) ( ((oflag) & O_CREAT) != 0 )
+#endif
+
 /* POSIX.1-2001 specifies that these types are defined by <fcntl.h>.
    Earlier POSIX standards permitted any type ending in `_t' to be defined
    by any POSIX header, so we don't conditionalize the definitions here.  */
@@ -160,7 +164,7 @@  typedef __pid_t pid_t;
 extern int fcntl (int __fd, int __cmd, ...);
 
 /* Open FILE and return a new file descriptor for it, or -1 on error.
-   OFLAG determines the type of access used.  If O_CREAT is on OFLAG,
+   OFLAG determines the type of access used.  If __OPEN_NEEDS_MODE(OFLAG),
    the third argument is taken as a `mode_t', the mode of the created file.
 
    This function is a cancellation point and therefore not marked with
diff --git a/io/open.c b/io/open.c
index 24aa38033984..4a00c3359e02 100644
--- a/io/open.c
+++ b/io/open.c
@@ -23,7 +23,7 @@ 
 #include <stdio.h>
 
 
-/* Open FILE with access OFLAG.  If OFLAG includes O_CREAT,
+/* Open FILE with access OFLAG.  If __OPEN_NEEDS_MODE(OFLAG),
    a third argument is the file protection.  */
 int
 __libc_open (file, oflag)
@@ -38,7 +38,7 @@  __libc_open (file, oflag)
       return -1;
     }
 
-  if (oflag & O_CREAT)
+  if (__OPEN_NEEDS_MODE(oflag))
     {
       va_list arg;
       va_start(arg, oflag);
diff --git a/io/open64.c b/io/open64.c
index 3f3d2e8bbd4b..e3f2630bcf06 100644
--- a/io/open64.c
+++ b/io/open64.c
@@ -21,7 +21,7 @@ 
 #include <stddef.h>
 #include <stdio.h>
 
-/* Open FILE with access OFLAG.  If OFLAG includes O_CREAT,
+/* Open FILE with access OFLAG.  If __OPEN_NEEDS_MODE(OFLAG),
    a third argument is the file protection.  */
 int
 __libc_open64 (file, oflag)
@@ -36,7 +36,7 @@  __libc_open64 (file, oflag)
       return -1;
     }
 
-  if (oflag & O_CREAT)
+  if (__OPEN_NEEDS_MODE(oflag))
     {
       va_list arg;
       va_start (arg, oflag);
diff --git a/io/open64_2.c b/io/open64_2.c
index 7cafbba4fcc4..b95b205d92de 100644
--- a/io/open64_2.c
+++ b/io/open64_2.c
@@ -22,8 +22,8 @@ 
 int
 __open64_2 (const char *file, int oflag)
 {
-  if (oflag & O_CREAT)
-    __fortify_fail ("invalid open64 call: O_CREAT without mode");
+  if (__OPEN_NEEDS_MODE(oflag))
+    __fortify_fail ("invalid open64 call: missing mode argument");
 
   return __open64 (file, oflag);
 }
diff --git a/io/open_2.c b/io/open_2.c
index 65d2c1c845bc..e405280eeb5a 100644
--- a/io/open_2.c
+++ b/io/open_2.c
@@ -22,8 +22,8 @@ 
 int
 __open_2 (const char *file, int oflag)
 {
-  if (oflag & O_CREAT)
-    __fortify_fail ("invalid open call: O_CREAT without mode");
+  if (__OPEN_NEEDS_MODE(oflag))
+    __fortify_fail ("invalid open call: missing mode argument");
 
   return __open (file, oflag);
 }
diff --git a/io/openat.c b/io/openat.c
index 2d822702af43..aff079eb9fd4 100644
--- a/io/openat.c
+++ b/io/openat.c
@@ -30,7 +30,7 @@  int __have_atfcts;
 #endif
 
 /* Open FILE with access OFLAG.  Interpret relative paths relative to
-   the directory associated with FD.  If OFLAG includes O_CREAT, a
+   the directory associated with FD.  If __OPEN_NEEDS_MODE(OFLAG), a
    third argument is the file protection.  */
 int
 __openat (fd, file, oflag)
@@ -60,7 +60,7 @@  __openat (fd, file, oflag)
 	}
     }
 
-  if (oflag & O_CREAT)
+  if (__OPEN_NEEDS_MODE(oflag))
     {
       va_list arg;
       va_start (arg, oflag);
diff --git a/io/openat64.c b/io/openat64.c
index c0c4e19589bc..23a27a8baefd 100644
--- a/io/openat64.c
+++ b/io/openat64.c
@@ -23,7 +23,7 @@ 
 #include <sys/stat.h>
 
 /* Open FILE with access OFLAG.  Interpret relative paths relative to
-   the directory associated with FD.  If OFLAG includes O_CREAT, a
+   the directory associated with FD.  If __OPEN_NEEDS_MODE(OFLAG), a
    third argument is the file protection.  */
 int
 __openat64 (fd, file, oflag)
@@ -53,7 +53,7 @@  __openat64 (fd, file, oflag)
 	}
     }
 
-  if (oflag & O_CREAT)
+  if (__OPEN_NEEDS_MODE(oflag))
     {
       va_list arg;
       va_start (arg, oflag);
diff --git a/io/openat64_2.c b/io/openat64_2.c
index 6cfea6a9aace..20e1ecbed8d8 100644
--- a/io/openat64_2.c
+++ b/io/openat64_2.c
@@ -22,8 +22,8 @@ 
 int
 __openat64_2 (int fd, const char *file, int oflag)
 {
-  if (oflag & O_CREAT)
-    __fortify_fail ("invalid openat64 call: O_CREAT without mode");
+  if (__OPEN_NEEDS_MODE(oflag))
+    __fortify_fail ("invalid openat64 call: missing mode argument");
 
   return __openat64 (fd, file, oflag);
 }
diff --git a/io/openat_2.c b/io/openat_2.c
index 9e38c142671c..947a5d65b09f 100644
--- a/io/openat_2.c
+++ b/io/openat_2.c
@@ -22,8 +22,8 @@ 
 int
 __openat_2 (int fd, const char *file, int oflag)
 {
-  if (oflag & O_CREAT)
-    __fortify_fail ("invalid openat call: O_CREAT without mode");
+  if (__OPEN_NEEDS_MODE(oflag))
+    __fortify_fail ("invalid openat call: missing mode argument");
 
   return __openat (fd, file, oflag);
 }
diff --git a/sysdeps/mach/hurd/open.c b/sysdeps/mach/hurd/open.c
index 7d9b2de70c38..5c3f40d4d505 100644
--- a/sysdeps/mach/hurd/open.c
+++ b/sysdeps/mach/hurd/open.c
@@ -22,7 +22,7 @@ 
 #include <hurd.h>
 #include <hurd/fd.h>
 
-/* Open FILE with access OFLAG.  If OFLAG includes O_CREAT,
+/* Open FILE with access OFLAG.  If __OPEN_NEEDS_MODE(OFLAG),
    a third argument is the file protection.  */
 int
 __libc_open (const char *file, int oflag, ...)
@@ -30,7 +30,7 @@  __libc_open (const char *file, int oflag, ...)
   mode_t mode;
   io_t port;
 
-  if (oflag & O_CREAT)
+  if (__OPEN_NEEDS_MODE(oflag))
     {
       va_list arg;
       va_start (arg, oflag);
diff --git a/sysdeps/mach/hurd/openat.c b/sysdeps/mach/hurd/openat.c
index 318cb229ef64..cc8d97389c20 100644
--- a/sysdeps/mach/hurd/openat.c
+++ b/sysdeps/mach/hurd/openat.c
@@ -26,7 +26,7 @@ 
 #include <hurd/fd.h>
 
 /* Open FILE with access OFLAG.  Interpret relative paths relative to
-   the directory associated with FD.  If OFLAG includes O_CREAT, a
+   the directory associated with FD.  If __OPEN_NEEDS_MODE(OFLAG), a
    third argument is the file protection.  */
 int
 __openat (fd, file, oflag)
@@ -37,7 +37,7 @@  __openat (fd, file, oflag)
   mode_t mode;
   io_t port;
 
-  if (oflag & O_CREAT)
+  if (__OPEN_NEEDS_MODE(oflag))
     {
       va_list arg;
       va_start (arg, oflag);
diff --git a/sysdeps/posix/open64.c b/sysdeps/posix/open64.c
index 64d192af9793..9d484884d467 100644
--- a/sysdeps/posix/open64.c
+++ b/sysdeps/posix/open64.c
@@ -19,14 +19,14 @@ 
 #include <stdarg.h>
 #include <sysdep-cancel.h>
 
-/* Open FILE with access OFLAG.  If OFLAG includes O_CREAT,
+/* Open FILE with access OFLAG.  If __OPEN_NEEDS_MODE(OFLAG),
    a third argument is the file protection.  */
 int
 __libc_open64 (const char *file, int oflag, ...)
 {
   int mode = 0;
 
-  if (oflag & O_CREAT)
+  if (__OPEN_NEEDS_MODE(oflag))
     {
       va_list arg;
       va_start (arg, oflag);
diff --git a/sysdeps/unix/sysv/linux/bits/fcntl-linux.h b/sysdeps/unix/sysv/linux/bits/fcntl-linux.h
index e224dab58be2..906c6189043f 100644
--- a/sysdeps/unix/sysv/linux/bits/fcntl-linux.h
+++ b/sysdeps/unix/sysv/linux/bits/fcntl-linux.h
@@ -100,6 +100,9 @@ 
 # define __O_TMPFILE   020200000
 #endif
 
+#define __OPEN_NEEDS_MODE(oflag) \
+  ( ((oflag) & O_CREAT) != 0 || ((oflag) & __O_TMPFILE) == __O_TMPFILE )
+
 #ifndef F_GETLK
 # ifndef __USE_FILE_OFFSET64
 #  define F_GETLK	5	/* Get record locking info.  */
diff --git a/sysdeps/unix/sysv/linux/dl-openat64.c b/sysdeps/unix/sysv/linux/dl-openat64.c
index 9d00b459a60d..fea11da41fcb 100644
--- a/sysdeps/unix/sysv/linux/dl-openat64.c
+++ b/sysdeps/unix/sysv/linux/dl-openat64.c
@@ -28,7 +28,7 @@  openat64 (dfd, file, oflag)
      const char *file;
      int oflag;
 {
-  assert ((oflag & O_CREAT) == 0);
+  assert (!__OPEN_NEEDS_MODE(oflag));
 
 #ifdef __NR_openat
   return INLINE_SYSCALL (openat, 3, dfd, file, oflag | O_LARGEFILE);
diff --git a/sysdeps/unix/sysv/linux/generic/open.c b/sysdeps/unix/sysv/linux/generic/open.c
index 4f73fa019cd8..2a4a219cfe31 100644
--- a/sysdeps/unix/sysv/linux/generic/open.c
+++ b/sysdeps/unix/sysv/linux/generic/open.c
@@ -22,14 +22,14 @@ 
 #include <stdio.h>
 #include <sysdep-cancel.h>
 
-/* Open FILE with access OFLAG.  If OFLAG includes O_CREAT,
+/* Open FILE with access OFLAG.  If __OPEN_NEEDS_MODE(OFLAG),
    a third argument is the file protection.  */
 int
 __libc_open (const char *file, int oflag, ...)
 {
   int mode = 0;
 
-  if (oflag & O_CREAT)
+  if (__OPEN_NEEDS_MODE(oflag))
     {
       va_list arg;
       va_start (arg, oflag);
@@ -59,7 +59,7 @@  __open_nocancel (const char *file, int oflag, ...)
 {
   int mode = 0;
 
-  if (oflag & O_CREAT)
+  if (__OPEN_NEEDS_MODE(oflag))
     {
       va_list arg;
       va_start (arg, oflag);
diff --git a/sysdeps/unix/sysv/linux/generic/open64.c b/sysdeps/unix/sysv/linux/generic/open64.c
index 93d79e381fab..4ad004bb7f67 100644
--- a/sysdeps/unix/sysv/linux/generic/open64.c
+++ b/sysdeps/unix/sysv/linux/generic/open64.c
@@ -22,14 +22,14 @@ 
 #include <stdio.h>
 #include <sysdep-cancel.h>
 
-/* Open FILE with access OFLAG.  If OFLAG includes O_CREAT,
+/* Open FILE with access OFLAG.  If __OPEN_NEEDS_MODE(OFLAG),
    a third argument is the file protection.  */
 int
 __libc_open64 (const char *file, int oflag, ...)
 {
   int mode = 0;
 
-  if (oflag & O_CREAT)
+  if (__OPEN_NEEDS_MODE(oflag))
     {
       va_list arg;
       va_start (arg, oflag);
diff --git a/sysdeps/unix/sysv/linux/open64.c b/sysdeps/unix/sysv/linux/open64.c
index 0d63806d04ae..fa1ea270cac5 100644
--- a/sysdeps/unix/sysv/linux/open64.c
+++ b/sysdeps/unix/sysv/linux/open64.c
@@ -21,14 +21,14 @@ 
 #include <stdio.h>
 #include <sysdep-cancel.h>
 
-/* Open FILE with access OFLAG.  If OFLAG includes O_CREAT,
+/* Open FILE with access OFLAG.  If __OPEN_NEEDS_MODE(OFLAG),
    a third argument is the file protection.  */
 int
 __libc_open64 (const char *file, int oflag, ...)
 {
   int mode = 0;
 
-  if (oflag & O_CREAT)
+  if (__OPEN_NEEDS_MODE(oflag))
     {
       va_list arg;
       va_start (arg, oflag);
diff --git a/sysdeps/unix/sysv/linux/openat.c b/sysdeps/unix/sysv/linux/openat.c
index 36555b958fdd..21748339ee1d 100644
--- a/sysdeps/unix/sysv/linux/openat.c
+++ b/sysdeps/unix/sysv/linux/openat.c
@@ -67,7 +67,7 @@  __OPENAT (fd, file, oflag)
      int oflag;
 {
   mode_t mode = 0;
-  if (oflag & O_CREAT)
+  if (__OPEN_NEEDS_MODE(oflag))
     {
       va_list arg;
       va_start (arg, oflag);