[PATCH v3] linux: Add openat2 (BZ 31664)

[Adhemerval Zanella Netto]

On 24/03/25 12:15, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> diff --git a/manual/llio.texi b/manual/llio.texi
>> index b6bc7f2210..786ac5b451 100644
>> --- a/manual/llio.texi
>> +++ b/manual/llio.texi
> 
>> +@table @code
>> +@item RESOLVE_NO_XDEV
>> +Disallow traversal of mount points during path resolution (including all
>> +bind mounts).
>> +@item RESOLVE_NO_MAGICLINKS
>> +Disallow all @strong{magic-link} resolution during path resolution. Magic
>> +links are simbolic link-like objects that are found in @strong{procfs};
> 
> typo: s[y]mbolic

Ack.

> 
>> +for example the @code{/proc/pid/exe}.
>> +@item RESOLVE_NO_SYMLINKS
>> +Disallow resolution of symbolic links during path resolution.
>> +This option implies @code{RESOLVE_NO_MAGICLINKS}.
>> +@item RESOLVE_BENEATH
>> +Do not permit the path resolution to succeed if any component of the
>> +resolution is not a descendant of the directory indicated by @code{dirfd}.
>> +@item RESOLVE_IN_ROOT
>> +Treat the directory referred to by @code{dirfd} as the root directory
>> +while resolving the @code{pathname}.
> 
> Maybe clarify how RESOLVE_BENEATH and RESOLVE_IN_ROOT are different?
> 
> I'm guessing, but "../" and "/" are ways for referring to the
> @var{dirfd} directory with RESOLVE_IN_ROOT, but are errors for with
> RESOLVE_BENEATH.

Yes, the RESOLVE_BENEATH disallows resolution if any component of the
path is not a descendant of @var{dirfd}.  It means that if have a
directory of:

  /tmp/tempdir
  |- test_dir_link -> /tmp
  |- test_dir_link_2 -> test_dir_link
  |- temp_dir_link -> /tmp/tempdir

calling openat2 with a dirfd referring to '/tmp/tempdir' will fail
for 'test_dir_link', 'test_dir_link_2', or 'temp_dir_link'.

I will add some more information on how each flags works.

> 
> I think you should use @var{pathname} and @var{dirfd} because they refer
> to the openat2 argument below.

Ack.

> 
>> +@item RESOLVE_CACHED
>> +Make the open operation fail unless all path components are already
>> +present in the kernel's lookup cache.
>> +@end table
>> +
>> +@end table
>> +
>> +For additional information, consult the manual page @manpageurl{openat2,2}.
>> +@xref{Linux Kernel}.
>> +@end deftp
>> +
>> +
>> +@deftypefun int openat2 (int @var{dirfd}, const char *@var{pathname}, struct open_how @var{how}, size_t @var{size})
> 
> Missing * for @var{how}.

Ack.

> 
>> +@standards{Linux, fcntl.h}
>> +@safety{@mtsafe{}@assafe{}@acsafe{}}
>> +This function is a extension of the @code{openat} and provides a superset of its
>> +functionality.  @xref{Descriptor-Relative Access}.
>> +
>> +The @code{size} define the expected size of @code{how} data structure.
>> +It is recommended to initialize unused fields to zero, either using
>> +@code{memset}, or using a structure initializer.
> 
> Does the kernel promise that there won't be any padding not covered by
> fields?  Otherwise applications really should use memset.

The initial commit (fddb5d430ad9fa91b49b1d34d0202ffe2fa0e179) that added
openat2 does have comment that the current types for open_how member
were chosen to avoid padding:

    open_how does not contain an embedded size field, because it is of
    little benefit (userspace can figure out the kernel open_how size at
    runtime fairly easily without it). It also only contains u64s (even
    though ->mode arguably should be a u16) to avoid having padding fields
    which are never used in the future.

So I take the idea is to indeed avoid padding in the future and allow 
struct initializer (it is also used on kernel selftests).

> 
>> +On failure, @code{openat2} returns @math{-1} and sets @code{errno}.  The
>> +following errors are related the way extensibility is handled.
> 
> The last sentence appears to apply to E2BIG

Ack, I will remove it.

> 
>> +@table @code
>> +@item E2BIG
>> +An extension that the kernel does support was specified in @code{how},
> 
> 
> @code{*@var{how}}

Ack.

> 
>> +or a larger struct was used with non-zero fields.
>> +@item EAGAIN
>> +@code{how.resolve} contains either RESOLVE_IN_ROOT or RESOLVE_BENEATH, and
> 
> @code{@var{how}->resolve}

Ack.

> 
> 
>> +the kernel could not ensure that @code{".."} component did not escape.  Or
>> +@code{RESOLVE_CACHED} was set, and the open operation cannot be performed
>> +using only cached information.
>> +@item EINVAL
>> +And unknown flag or invalid value was used on @code{how}; or @code{mode}
> 
> @code{*@var{how}}
> 
> @code{@var{how}->mode}

Ack.

> 
>> +is non-zero, but @code{how.flags} does not contain @code{O_CREAT} or
> 
> @code{@var{how}->flags}

Ack.

> 
>> +@code{O_TMPFILE}, or @code{size} is smaller than the ones supported
> 
> @var{size}

Ack.

> 
>> +by the kernel.
>> +@end table
>> +
>> +It can also return all the errors @code{openat} returns, or other errors
>> +due new fields added by the kernel.
>> +
>> +Similar to @code{openat}, @code{openat2} is a cancellation point.
>> +
>> +@strong{NB:} Different than other open-like functions, the kernel only
>> +provides the LFS variant.  When the sources are translated with
>> +@code{_FILE_OFFSET_BITS == 64} this function are not routed a different
> 
> typo: this function [is] not routed

Ack.

> 
>> +symbol.
>> +@end deftypefun
>> +
>> +
>>  @deftypefn {Obsolete function} int creat (const char *@var{filename}, mode_t @var{mode})
>>  @standards{POSIX.1, fcntl.h}
>>  @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{@acsfd{}}}
> 
>> diff --git a/sysdeps/unix/sysv/linux/tst-openat2.c b/sysdeps/unix/sysv/linux/tst-openat2.c
>> new file mode 100644
>> index 0000000000..34b0a87ced
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/tst-openat2.c
>> @@ -0,0 +1,259 @@
>> +/* Linux openat2 tests.
>> +   Copyright (C) 2025 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <https://www.gnu.org/licenses/>.  */
>> +
>> +/* openat2 always return a file descriptor in LFS mode.  */
>> +#define _FILE_OFFSET_BITS 64
> 
> Should we really define this?  We should check that the wrapper works
> for non-LFS builds, too.
> 
> If we document the RESOLVE_BENEATH and RESOLVE_IN_ROOT difference, we
> should have test case for that.

Ack.

> 
>> +  for (struct struct_test *t = tests; t != array_end (tests); t++)
>> +    {
>> +      int fd = openat2 (AT_FDCWD, ".", (struct open_how *) &t->arg, t->size);
>> +      if (fd == -1 && errno == ENOSYS)
>> +	FAIL_UNSUPPORTED ("openat2 is not supported by the kernel");
> 
> This should check if any successes have been observed.  We do not want
> to flag the test as UNSUPPORTED in case some strange kernel decides to
> signal lack of FS support through ENOSYS (as it happen with
> copy_file_range).
Fair enough, I will add a early test for openat2 existence.