Re: [PATCH RFC 3/3] openat2.2: document new openat2(2) syscall

Aleksa Sarai
2019-10-09T10:17:55Z

[-- Attachment #1: Type: text/plain, Size: 22645 bytes --]

On 2019-10-09, Michael Kerrisk (man-pages) <mtk.manpages@gmail.com> wrote:
> Hello Aleksa,
>
> Thanks for this. It's a great piece of documentation work!
>
> I would prefer the path_resolution(7) piece as a separate patch.

Thanks, and will do.

> On 10/3/19 4:55 PM, Aleksa Sarai wrote:
> > Rather than trying to merge the new syscall documentation into open.2
> > (which would probably result in the man-page being incomprehensible),
> > instead the new syscall gets its own dedicated page with links between
> > open(2) and openat2(2) to avoid duplicating information such as the list
> > of O_* flags or common errors.
>
> Yes, looking at the size of the proposed openat2(2) page,
> this seems best.
> >
> > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> > ---
> > man2/open.2 | 5 +
> > man2/openat2.2 | 381 +++++++++++++++++++++++++++++++++++++++++
> > man7/path_resolution.7 | 57 ++++--
> > 3 files changed, 426 insertions(+), 17 deletions(-)
> > create mode 100644 man2/openat2.2
> >
> > diff --git a/man2/open.2 b/man2/open.2
> > index 7217fe056e5e..a0b43394bbee 100644
> > --- a/man2/open.2
> > +++ b/man2/open.2
> > @@ -65,6 +65,10 @@ open, openat, creat \- open and possibly create a file
> > .BI "int openat(int " dirfd ", const char *" pathname ", int " flags );
> > .BI "int openat(int " dirfd ", const char *" pathname ", int " flags \
> > ", mode_t " mode );
> > +.PP
> > +/* Docuented separately, in \fBopenat2\fP(2). */
>
> Documented
>
> > +.BI "int openat2(int " dirfd ", const char *" pathname ", \
> > +const struct open_how *" how ", size_t " size ");
> > .fi
> > .PP
> > .in -4n
> > @@ -1808,6 +1812,7 @@ will create a regular file (i.e.,
> > .B O_DIRECTORY
> > is ignored).
> > .SH SEE ALSO
> > +.BR openat2 (2),
>
> Entries here should into alphabetical order (within
> sections).
>
> > .BR chmod (2),
> > .BR chown (2),
> > .BR close (2),
> > diff --git a/man2/openat2.2 b/man2/openat2.2
> > new file mode 100644
> > index 000000000000..c43c76046243
> > --- /dev/null
> > +++ b/man2/openat2.2
> > @@ -0,0 +1,381 @@
> > +.\" Copyright (C) 2019 Aleksa Sarai <cyphar@cyphar.com>
> > +.\"
> > +.\" %%%LICENSE_START(VERBATIM)
> > +.\" Permission is granted to make and distribute verbatim copies of this
> > +.\" manual provided the copyright notice and this permission notice are
> > +.\" preserved on all copies.
> > +.\"
> > +.\" Permission is granted to copy and distribute modified versions of this
> > +.\" manual under the conditions for verbatim copying, provided that the
> > +.\" entire resulting derived work is distributed under the terms of a
> > +.\" permission notice identical to this one.
> > +.\"
> > +.\" Since the Linux kernel and libraries are constantly changing, this
> > +.\" manual page may be incorrect or out-of-date. The author(s) assume no
> > +.\" responsibility for errors or omissions, or for damages resulting from
> > +.\" the use of the information contained herein. The author(s) may not
> > +.\" have taken the same level of care in the production of this manual,
> > +.\" which is licensed free of charge, as they might when working
> > +.\" professionally.
> > +.\"
> > +.\" Formatted or processed versions of this manual, if unaccompanied by
> > +.\" the source, must acknowledge the copyright and authors of this work.
> > +.\" %%%LICENSE_END
> > +.TH OPENAT2 2 2019-10-03 "Linux" "Linux Programmer's Manual"
> > +.SH NAME
> > +openat2 \- open and possibly create a file (extended)
> > +.SH SYNOPSIS
> > +.nf
> > +.B #include <sys/types.h>
> > +.B #include <sys/stat.h>
> > +.B #include <fcntl.h>
> > +.PP
> > +.BI "int openat2(int " dirfd ", const char *" pathname ", \
> > +const struct open_how *" how ", size_t " size ");
> > +.fi
> > +.PP
> > +.IR Note :
> > +There is no glibc wrapper for this system call; see NOTES.
> > +.SH DESCRIPTION
> > +The
> > +.BR openat2 ()
> > +system call is an extension of
> > +.BR openat (2)
> > +and provides a superset of its functionality. Rather than taking a single
>
> Please start new sentences on new source lines. I recently added this
> text in man-pages(7):
>
> Use semantic newlines
> In the source of a manual page, new sentences should be started on
> new lines, and long sentences should split into lines at clause
> breaks (commas, semicolons, colons, and so on). This convention,
> sometimes known as "semantic newlines", makes it easier to see the
> effect of patches, which often operate at the level of individual
> sentences or sentence clauses.
>
> > +.I flag
> > +argument, an extensible structure (\fIhow\fP) is passed instead to allow for
> > +seamless future extensions.
>
> s/seamless//
>
> > +.PP
> > +.I size
> > +must be set to
> > +.IR "sizeof(struct open_how)" ,
> > +to facilitate future extensions (see the "Extensibility" section of the
> > +\fBNOTES\fP for more detail on how extensions are handled.)
> > +
> > +.SS The open_how structure
> > +The following structure indicates how
> > +.I pathname
> > +should be opened, and acts as a superset of the
> > +.IR flag " and " mode
> > +arguments to
> > +.BR openat (2).
> > +.PP
> > +.in +4n
> > +.EX
> > +struct open_how {
> > + uint32_t flags; /* open(2)-style O_* flags. */
> > + union {
> > + uint16_t mode; /* File mode bits for new file creation. */
> > + uint16_t upgrade_mask; /* Restrict how O_PATHs may be re-opened. */
> > + };
> > + uint32_t resolve; /* RESOLVE_* path-resolution flags. */
> > +};
> > +.EE
> > +.in
> > +.PP
> > +Any future extensions to
> > +.BR openat2 ()
> > +will be implemented as new fields appended to the above structure, with the
> > +zero value of the new fields acting as though the extension were not present.
> > +.PP
> > +The meaning of each field is as follows:
> > +.RS
> > +
> > +.I flags
> > +.RSall
> > +The file creation and status flags to use for this operation. All of the
> > +.B O_*
> > +flags defined for
> > +.BR openat (2)
> > +are valid
> > +.BR openat2 ()
> > +flag values.
> > +.RE
> > +
> > +.I upgrade_mask
> > +.RS
> > +Restrict with which
> > +.I access modes
> > +the returned
> > +.B O_PATH
> > +descriptor may be re-opened (either through
> > +.B O_EMPTYPATH
> > +or
> > +.IR /proc/self/fd/ .)
> > +This field may only be set to a non-zero value if
> > +.I flags
> > +contains
> > +.BR O_PATH .
> > +By default, an
> > +.B O_PATH
> > +file descriptor of an ordinary file may be re-opened with with any access mode (but an
> > +.B O_PATH
> > +file descriptor of a magic-link may only be re-opened with access modes that
> > +the original magic-link possessed). The full list of
>
> magic link (throughout the page)
>
> > +.I upgrade_mask
> > +flags is given below.
> > +.TP
> > +.B UPGRADE_NOREAD
> > +Do not permit the
> > +.B O_PATH
> > +file descriptor to be re-opened for reading (i.e.
> > +.BR O_RDONLY " or " O_RDWR .)
> > +.TP
> > +.B UPGRADE_NOWRITE
> > +Do not permit the
> > +.B O_PATH
> > +file descriptor to be re-opened for writing (i.e.
> > +.BR O_WRONLY ", " O_RDWR ", or " O_APPEND .)
> > +.RE
> > +.I resolve
> > +.RS
> > +Change how the components of
> > +.I pathname
> > +will be resolved (see
> > +.BR path_resolution (7)
> > +for background information.) The primary use-case for these flags is to allow
>
> use case
>
> > +trusted programs to restrict how un-trusted paths (or paths inside un-trusted
>
> untrusted
>
> > +directories) are resolved. The full list of
> > +.I resolve
> > +flags is given below.
> > +.TP
> > +.B RESOLVE_NO_XDEV
> > +Disallow all mount-point crossings during path resolution (including
>
> I think better would be: "Disallow traversal of mount points". Do you
> agree?

Yes, that sounds better.

> > +all bind-mounts).
>
> bind mounts
>
> > +
> > +Users of this flag are encouraged to make its use configurable (unless it is
> > +used for a specific security purpose), as bind-mounts are very widely used by
> > +end-users and thus enabling this flag globally may result in spurious errors on
> > +some systems.
> > +.TP
> > +.B RESOLVE_NO_SYMLINKS
> > +Disallow all symlink resolution during path resolution. If the trailing
>
> Disallow resolution of symbolic links during path resolution
>
> > +component is a symlink, and
>
> symbolic link (throughout the page)
>
> > +.I flags
> > +contains both
> > +.BR O_PATH " and " O_NOFOLLOW ","
> > +then an
> > +.B O_PATH
> > +file descriptor referencing the symlink will be returned. This option implies
> > +.BR RESOLVE_NO_MAGICLINKS .
> > +
> > +Users of this flag are encouraged to make its use configurable (unless it is
> > +used for a specific security purpose), as symlinks are very widely used by
> > +end-users and thus enabling this flag globally may result in spurious errors on
> > +some systems.
>
> It's not really clear what you mean by "enabling this flag globally".
> Could you reword, or explain in a bit more detail?

A better word might be "indiscriminately" -- the point being that if
a program uses it for every openat2() call (and users cannot disable
it), then the program will break on all sorts of systems.

> > +.TP
> > +.B RESOLVE_NO_MAGICLINKS
> > +Disallow all magic-link resolution during path resolution. If the trailing
> > +component is a magic-link, and
> > +.I flags
> > +contains both
> > +.BR O_PATH " and " O_NOFOLLOW ","
> > +then an
> > +.B O_PATH
> > +file descriptor referencing the magic-link will be returned.
> > +
> > +Magic-links are symlink-like objects that are most notably found in
> > +.BR proc (5)
> > +(examples include
> > +.IR /proc/[pid]/exe " and " /proc/[pid]/fd/* .)
> > +Due to the potential danger of unknowingly opening these magic-links, it may be
> > +preferable for users to disable their resolution entirely (see
> > +.BR symlink (7)
> > +for more details.)
> > +.TP
> > +.B 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
> > +.IR dirfd .
> > +This results in absolute symlinks (and absolute values of
> > +.IR pathname )
> > +to be rejected. Magic-link resolution is also not permitted.
>
> So, this flag implies RESOLVE_NO_MAGICLINKS? If yes,
> it would be good to state that more explicitly,

It does, though this might change in the future (some magic-link
resolutions might be safe -- but it's unclear what the semantics should
be). Users should explicitly set RESOLVE_NO_MAGICLINKS if they really
don't want to resolve them.

> > +
> > +.TP
> > +.B RESOLVE_IN_ROOT
> > +Temporarily treat
> > +.I dirfd
> > +as the root of the filesystem (as though the user called
>
> Perhaps better:
>
> Treat
> .I dirfd
> as the root directory while resolving
> .I pathname
> (as though...)

Yeah that sounds better.

> > +.BR chroot (2)
> > +with
> > +.IR dirfd
> > +as the argument.) Absolute symlinks and ".." path components will be scoped to
> > +.IR dirfd . Magic-link resolution is also not permitted.
>
> Insert a newline before "Magic" to fix a formatting problem.
>
> So, this flag implies RESOLVE_NO_MAGICLINKS? If yes,
> it would be good to state that more explicitly,

Same reply as above.

> > +
> > +However, unlike
> > +.BR chroot (2)
> > +(which changes the filesystem root persistently for an entire thread-group),
>
> s/persistently for an entire thread-group/
> /permanently for a process/
>
> > +.B RESOLVE_IN_ROOT
> > +allows a program to efficiently restrict path resolution for only certain
> > +operations. It also has several hardening features (such as not permitting
> > +magic-link resolution) which
> > +.BR chroot (2)
> > +does not.
> > +.RE
> > +
> > +.RE
> > +
> > +.PP
> > +Unlike
> > +.BR openat (2),
> > +any unknown flags set in fields of
> > +.I how
> > +will result in an error, rather than being ignored.
>
> Thank you, thank you, thank you. It was sad
> that openat() never fixed that antifeature.

No problem, it's bothered me for a long time as well. :D

> > In addition, an error will
> > +be returned if the value of the
> > +.IR mode " and " upgrade_mask
> > +union is non-zero unless:
> > +.RS
> > +.IP * 3
> > +.I flags
> > +indicates that a new file will be created (it contains
> > +.BR O_CREAT " or " O_TMPFILE ),
> > +in which case
> > +.I mode
> > +may be any valid file mode.
> > +.IP *
> > +.I flags
> > +contains
> > +.BR O_PATH ,
> > +in which case
> > +.I upgrade_mask
> > +must only contain valid
> > +.B UPGRADE_*
> > +flags.
> > +.RE
> > +
> > +.SH RETURN VALUE
> > +On success, a new file descriptor is returned. On error, -1 is returned, and
> > +.I errno
> > +is set appropriately.
> > +
> > +.SH ERRORS
> > +The set of errors returned by
> > +.BR openat2 ()
> > +includes all of the errors returned by
> > +.BR openat (2),
> > +as well as the following additional errors:
> > +.TP
> > +.B EINVAL
> > +An unknown flag or invalid value was specified in
> > +.IR how .
> > +.TP
> > +.B EINVAL
> > +.I size
> > +was smaller than any known version of
> > +.IR "struct open_how" .
> > +.TP
> > +.B E2BIG
> > +An extension was specified in
> > +.IR how ,
> > +which the current kernel does not support (see the "Extensibility" section of
> > +the \fBNOTES\fP for more detail on how extensions are handled.)
> > +.TP
> > +.B EAGAIN
> > +.I resolve
> > +contains either
> > +.BR RESOLVE_IN_ROOT " or " RESOLVE_BENEATH ,
> > +and the kernel could not ensure that a ".." component didn't escape (due to a
> > +race condition or potential attack). Callers may choose to retry the
> > +.BR openat2 ()
> > +call.
> > +.TP
> > +.B EXDEV
> > +.I resolve
> > +contains either
> > +.BR RESOLVE_IN_ROOT " or " RESOLVE_BENEATH ,
> > +and a path component attempted to escape the root of the resolution.
> > +
> > +.TP
> > +.B EXDEV
> > +.I resolve
> > +contains
> > +.BR RESOLVE_NO_XDEV ,
> > +and a path component attempted to cross a mount-point.
>
> mount point
>
> > +
> > +.TP
> > +.B ELOOP
> > +.I resolve
> > +contains
> > +.BR RESOLVE_NO_SYMLINKS ,
> > +and one of the path components was a symlink.
> > +.TP
> > +.B ELOOP
> > +.I resolve
> > +contains
> > +.BR RESOLVE_NO_MAGICLINKS ,
> > +and one of the path components was a magic-link.
> > +
> > +.SH VERSIONS
> > +.BR openat2 ()
> > +was added to Linux in kernel 5.FOO.
> > +
> > +.SH CONFORMING TO
> > +This system call is Linux-specific.
> > +
> > +The semantics of
> > +.B RESOLVE_BENEATH
> > +were modelled after FreeBSD's
> > +.BR O_BENEATH .
> > +
> > +.SH NOTES
> > +Glibc does not provide a wrapper for this system call; call it using
> > +.BR syscall (2).
> > +
> > +.SS Extensibility
> > +In order to allow for
> > +.I struct open_how
> > +to be extended in future kernel revisions,
> > +.BR openat2 ()
> > +requires userspace to specify what sized
>
> s/what sized/the size of/
>
> > +.I struct open_how
> > +structure they are passing. By providing this information, it is possible for
> > +.BR openat2 ()
> > +to provide both forwards- and backwards-compatibility \(em with
> > +.I size
> > +acting as an implicit version number (because new extension fields will always
> > +be appended, the size will always increase.) This extensibility design is very
> > +similar to other system calls such as
> > +.BR perf_setattr "(2), " perf_event_open "(2), and " clone (3).
>
> The following explantion of uszie and ksize is great. Thanks for that.

Glad to hear you don't think it's too much fluff. :D

> > +If we let
> > +.I usize
> > +be the size of the structure according to userspace and
> > +.I ksize
> > +be the size of the structure which the kernel supports, then there are only
> > +three cases to consider:
> > +
> > +.RS
> > +.IP * 3
> > +If
> > +.IR ksize " equals " usize ,
> > +then there is no version mismatch and
> > +.I how
> > +can be used verbatim.
> > +.IP *
> > +If
> > +.IR ksize " is larger than " usize ,
> > +then there are some extensions the kernel supports which the userspace program
> > +is unaware of. Because all extensions must have their zero values be a no-op,
> > +the kernel treats all of the extension fields not set by userspace to have zero
> > +values. This provides backwards-compatibility.
> > +.IP *
> > +If
> > +.IR ksize " is smaller than " usize ,
> > +then there are some extensions which the userspace program is aware of but the
> > +kernel does not support. Because all extensions must have their zero values be
> > +a no-op, the kernel can safely ignore the unsupported extension fields if they
> > +are all-zero. If any unsupported extension fields are non-zero, then an error
> > +is returned. This provides forwards-compatibility.
> > +.RE
> > +
> > +Therefore, most userspace programs will not need to have any special handling
> > +of extensions. However, if a userspace program wishes to determine what
> > +extensions the running kernel supports, they may conduct a binary search on
> > +.IR size
> > +(to find the largest value which doesn't produce an error.)
> > +
> > +.SH SEE ALSO
> > +.BR openat (2),
> > +.BR path_resolution (7),
> > +.BR symlink (7)
> > diff --git a/man7/path_resolution.7 b/man7/path_resolution.7
> > index 85dd354e9a93..3da3e5b614c8 100644
> > --- a/man7/path_resolution.7
> > +++ b/man7/path_resolution.7
> > @@ -29,17 +29,17 @@ path_resolution \- how a pathname is resolved to a file
> > Some UNIX/Linux system calls have as parameter one or more filenames.
> > A filename (or pathname) is resolved as follows.
> > .SS Step 1: start of the resolution process
> > -If the pathname starts with the \(aq/\(aq character,
> > -the starting lookup directory
> > -is the root directory of the calling process.
> > -(A process inherits its
> > -root directory from its parent.
> > -Usually this will be the root directory
> > -of the file hierarchy.
> > -A process may get a different root directory
> > -by use of the
> > +If the pathname starts with the \(aq/\(aq character, the starting lookup
> > +directory is the root directory of the calling process. (A process inherits its
> > +root directory from its parent. Usually this will be the root directory of the
> > +file hierarchy. A process may get a different root directory by use of the
> > .BR chroot (2)
> > -system call.
> > +system call, or may temporarily use a different root directory by using
> > +.BR openat2 (2)
> > +with the
> > +.B RESOLVE_IN_ROOT
> > +flag set.
> > +.PP
> > A process may get an entirely private mount namespace in case
> > it\(emor one of its ancestors\(emwas started by an invocation of the
> > .BR clone (2)
> > @@ -48,16 +48,24 @@ system call that had the
> > flag set.)
> > This handles the \(aq/\(aq part of the pathname.
> > .PP
> > -If the pathname does not start with the \(aq/\(aq character, the
> > -starting lookup directory of the resolution process is the current working
> > -directory of the process.
> > -(This is also inherited from the parent.
> > -It can be changed by use of the
> > +If the pathname does not start with the \(aq/\(aq character, the starting
> > +lookup directory of the resolution process is the current working directory of
> > +the process \(em or in the case of
> > +.BR openat (2)-style
> > +syscalls, the
>
> system calls
>
> > +.I dfd
> > +argument (or the current working directory if
> > +.B AT_FDCWD
> > +is passed as the
> > +.I dfd
> > +argumnet). The current working directory is inherited from the parent, and can
>
> argument
>
> > +be changed by use of the
> > .BR chdir (2)
> > -system call.)
> > +syscall.
>
> "system call" please.
>
> > .PP
> > Pathnames starting with a \(aq/\(aq character are called absolute pathnames.
> > Pathnames not starting with a \(aq/\(aq are called relative pathnames.
> > +
>
> No blank line here.
>
> > .SS Step 2: walk along the path
> > Set the current lookup directory to the starting lookup directory.
> > Now, for each nonfinal component of the pathname, where a component
> > @@ -124,6 +132,13 @@ the kernel's pathname-resolution code
> > was reworked to eliminate the use of recursion,
> > so that the only limit that remains is the maximum of 40
> > resolutions for the entire pathname.
> > +.PP
> > +The resolution of syscalls during this stage can be blocked by using
>
> "resolution of syscall" seems wrong? "syscall" should be something
> else?

Yeah, should be "resolution of symlinks". ;)

> > +.BR openat2 (2),
> > +with the
> > +.B RESOLVE_NO_SYMLINKS
> > +flag set.
> > +
> > .SS Step 3: find the final entry
> > The lookup of the final component of the pathname goes just like
> > that of all other components, as described in the previous step,
> > @@ -160,7 +175,8 @@ The path resolution process will assume that these entries have
> > their conventional meanings, regardless of whether they are
> > actually present in the physical filesystem.
> > .PP
> > -One cannot walk down past the root: "/.." is the same as "/".
> > +One cannot walk up past the root: "/.." is the same as "/".
> > +
>
> No blank line please.
>
> > .SS Mount points
> > After a "mount dev path" command, the pathname "path" refers to
> > the root of the filesystem hierarchy on the device "dev", and no
> > @@ -169,6 +185,13 @@ longer to whatever it referred to earlier.
> > One can walk out of a mounted filesystem: "path/.." refers to
> > the parent directory of "path",
> > outside of the filesystem hierarchy on "dev".
> > +.PP
> > +Mount-point crossings can be blocked by using
>
> Traversal of mount points can be disallowed by...
>
> > +.BR openat2 (2),
> > +with the
> > +.B RESOLVE_NO_XDEV
> > +flag set (though note that this also restricts bind-mount crossings).
> > +
>
> No blank line please.
>
> > .SS Trailing slashes
> > If a pathname ends in a \(aq/\(aq, that forces resolution of the preceding
> > component as in Step 2: it has to exist and resolve to a directory.
> >

Thanks so much, and I'll clean up your nits.

--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]