Re: [PATCH 2/2] fcntl: drop nonnull attribute for openat, openat2's path argument [BZ #34313]

enh <[email protected]> writes:

> is this actually true? i don't have a 7.2 kernel to test on, but
> looking at the source it looks like we wouldn't get as far as

I think you're right, but I'm also not sure it matters much. Dropping
the attribute isn't a promise that it's safe or anything (a wrong
nonnull attribute becomes very painful later on as gcc optimises based
on it). I wouldn't be surprised if O_EMPTYPATH was made like
AT_EMPTY_PATH.

Maybe that's not going to happen though.

>
> commit 5b313bcb6e3597dacd893ae9545fd087df46db45
> Author: Al Viro <[email protected]>
> Date:   Sat Oct 19 20:32:39 2024 -0400
>
>     teach filename_lookup() to treat NULL filename as ""
>
>     Signed-off-by: Al Viro <[email protected]>
>
> because we'd hit the failure path in
>
> static struct filename *
> do_getname(const char __user *filename, int flags, bool incomplete)
> {
>         struct filename *result;
>         char *kname;
>         int len;
>
>         result = alloc_filename();
>         if (unlikely(!result))
>                 return ERR_PTR(-ENOMEM);
>
>         /*
>          * First, try to embed the struct filename inside the names_cache
>          * allocation
>          */
>         kname = (char *)result->iname;
>         result->name = kname;
>
>         len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
>         /*
>          * Handle both empty path and copy failure in one go.
>          */
>         if (unlikely(len <= 0)) {
>                 /* The empty path is special. */
>                 if (!len && !(flags & LOOKUP_EMPTY))
>                         len = -ENOENT;
>         }
>
>         /*
>          * Uh-oh. We have a name that's approaching PATH_MAX. Allocate a
>          * separate struct filename so we can dedicate the entire
>          * names_cache allocation for the pathname, and re-do the copy from
>          * userland.
>          */
>         if (unlikely(len == EMBEDDED_NAME_MAX))
>                 len = getname_long(result, filename);
>         if (unlikely(len < 0)) {
>                 free_filename(result);
>                 return ERR_PTR(len);
>         }
>
>         initname(result);
>         if (likely(!incomplete))
>                 audit_getname(result);
>         return result;
> }
>
> first?
>
> (afaict the upstream kernel tests for these two syscalls also only
> test "" and not NULL.)
>
> On Sat, Aug 29, 2026 at 12:38 PM Sam James <[email protected]> wrote:
>>
>> .. and openat64.
>>
>> Linux 7.2 (31cf44efa6df72a524b40adefb80539f3a4e13ba) allows openat, openat2
>> to take a NULL path with the new O_EMPTYPATH flag, so the nonnull attribute
>> is no longer sound. Drop it.
>>
>> Bug: https://sourceware.org/PR34313
>> ---
>>  include/fcntl.h                                    |  6 ++----
>>  io/bits/fcntl2.h                                   | 12 ++++--------
>>  io/fcntl.h                                         |  8 +++-----
>>  sysdeps/unix/sysv/linux/bits/fcntl-linux-fortify.h |  2 +-
>>  sysdeps/unix/sysv/linux/bits/fcntl-linux.h         |  2 +-
>>  5 files changed, 11 insertions(+), 19 deletions(-)
>>
>> diff --git a/include/fcntl.h b/include/fcntl.h
>> index be435047bc..86890a5b3d 100644
>> --- a/include/fcntl.h
>> +++ b/include/fcntl.h
>> @@ -20,11 +20,9 @@ extern int __fcntl (int __fd, int __cmd, ...);
>>  libc_hidden_proto (__fcntl)
>>  extern int __fcntl64 (int __fd, int __cmd, ...) attribute_hidden;
>>  libc_hidden_proto (__fcntl64)
>> -extern int __openat (int __fd, const char *__file, int __oflag, ...)
>> -  __nonnull ((2));
>> +extern int __openat (int __fd, const char *__file, int __oflag, ...);
>>  libc_hidden_proto (__openat)
>> -extern int __openat64 (int __fd, const char *__file, int __oflag, ...)
>> -  __nonnull ((2));
>> +extern int __openat64 (int __fd, const char *__file, int __oflag, ...);
>>  libc_hidden_proto (__openat64)
>>
>>  extern int __open_2 (const char *__path, int __oflag);
>> diff --git a/io/bits/fcntl2.h b/io/bits/fcntl2.h
>> index f076cc3367..074c16b204 100644
>> --- a/io/bits/fcntl2.h
>> +++ b/io/bits/fcntl2.h
>> @@ -135,18 +135,14 @@ open64 (__fortify_clang_overload_arg (const char *, ,__path), int __oflag,
>>
>>  #ifdef __USE_ATFILE
>>  # ifndef __USE_FILE_OFFSET64
>> -extern int __openat_2 (int __fd, const char *__path, int __oflag)
>> -     __nonnull ((2));
>> +extern int __openat_2 (int __fd, const char *__path, int __oflag);
>>  extern int __REDIRECT (__openat_alias, (int __fd, const char *__path,
>> -                                       int __oflag, ...), openat)
>> -     __nonnull ((2));
>> +                                       int __oflag, ...), openat);
>>  # else
>>  extern int __REDIRECT (__openat_2, (int __fd, const char *__path,
>> -                                   int __oflag), __openat64_2)
>> -     __nonnull ((2));
>> +                                   int __oflag), __openat64_2);
>>  extern int __REDIRECT (__openat_alias, (int __fd, const char *__path,
>> -                                       int __oflag, ...), openat64)
>> -     __nonnull ((2));
>> +                                       int __oflag, ...), openat64);
>>  # endif
>>
>>  # ifdef __va_arg_pack_len
>> diff --git a/io/fcntl.h b/io/fcntl.h
>> index d0ad4d6652..3d90cc9adb 100644
>> --- a/io/fcntl.h
>> +++ b/io/fcntl.h
>> @@ -230,19 +230,17 @@ extern int open64 (const char *__file, int __oflag, ...) __nonnull ((1));
>>     This function is a cancellation point and therefore not marked with
>>     __THROW.  */
>>  # ifndef __USE_FILE_OFFSET64
>> -extern int openat (int __fd, const char *__file, int __oflag, ...)
>> -     __nonnull ((2));
>> +extern int openat (int __fd, const char *__file, int __oflag, ...);
>>  # else
>>  #  ifdef __REDIRECT
>>  extern int __REDIRECT (openat, (int __fd, const char *__file, int __oflag,
>> -                               ...), openat64) __nonnull ((2));
>> +                               ...), openat64);
>>  #  else
>>  #   define openat openat64
>>  #  endif
>>  # endif
>>  # ifdef __USE_LARGEFILE64
>> -extern int openat64 (int __fd, const char *__file, int __oflag, ...)
>> -     __nonnull ((2));
>> +extern int openat64 (int __fd, const char *__file, int __oflag, ...);
>>  # endif
>>  #endif
>>
>> diff --git a/sysdeps/unix/sysv/linux/bits/fcntl-linux-fortify.h b/sysdeps/unix/sysv/linux/bits/fcntl-linux-fortify.h
>> index 4c8f3a874e..b12b5b75da 100644
>> --- a/sysdeps/unix/sysv/linux/bits/fcntl-linux-fortify.h
>> +++ b/sysdeps/unix/sysv/linux/bits/fcntl-linux-fortify.h
>> @@ -25,7 +25,7 @@
>>  extern int __REDIRECT (__openat2_alias, (int __dfd, const char *__filename,
>>                                          const struct open_how *__how,
>>                                          size_t __usize), openat2)
>> -     __nonnull ((2, 3));
>> +     __nonnull ((3));
>>
>>  #if !__fortify_use_clang
>>  __errordecl (__openat2_invalid_size,
>> diff --git a/sysdeps/unix/sysv/linux/bits/fcntl-linux.h b/sysdeps/unix/sysv/linux/bits/fcntl-linux.h
>> index 587b815124..6aaa6cc4e7 100644
>> --- a/sysdeps/unix/sysv/linux/bits/fcntl-linux.h
>> +++ b/sysdeps/unix/sysv/linux/bits/fcntl-linux.h
>> @@ -485,7 +485,7 @@ extern int open_by_handle_at (int __mountdirfd, struct file_handle *__handle,
>>  extern int openat2 (int __dfd, const char * __filename,
>>                     const struct open_how * __how,
>>                     __SIZE_TYPE__ __usize)
>> -     __nonnull ((2, 3));
>> +     __nonnull ((3));
>>
>>  #endif /* use GNU */
>>
>> --
>> 2.55.0
>>

-----BEGIN PGP SIGNATURE-----

iQEBBAEWCgCpFiEEJaa7iN2bdkxrVUHCc4QJ9SDfkZAFAmqXQ7AbFIAAAAAABAAO
bWFudTIsMi41KzEuMTIsMiwyXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25z
Lm9wZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXQyNUE2QkI4OEREOUI3NjRDNkI1NTQx
QzI3Mzg0MDlGNTIwREY5MTkwDxxzYW1AZ2VudG9vLm9yZwAKCRBzhAn1IN+RkC5Q
AP9iRF7pEcUVzuEK1hNGCQhluMvINA5XAN4YM+V1BIZ0+QD/d70VfuhrauipjPLG
w/d4I8LodH4eTRawY/VH8MS+kwA=
=YSdC
-----END PGP SIGNATURE-----