LDP
/
man-pages
mirror of https://github.com/mkerrisk/man-pages
0
1
Fork 0
Code Releases Activity

mount_setattr.2: srcfix: add note explaining Christian's use of -ve dirfd values 

From email with Christian Brauner:

>>>>>>           int fd_tree = open_tree(-EBADF, source,
>>>>>>                        OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC |
>>>>>>                        AT_EMPTY_PATH | (recursive ? AT_RECURSIVE : 0));
>>>>>
>>>>> ???
>>>>> What is the significance of -EBADF here? As far as I can tell, it
>>>>> is not meaningful to open_tree()?
>>>>
>>>> I always pass -EBADF for similar reasons to [2]. Feel free to just use -1.
>>>
>>> ????
>>> But here, both -EBADF and -1 seem to be wrong. This argument
>>> is a dirfd, and so should either be a file descriptor or the
>>> value AT_FDCWD, right?
>>
>> [1]: In this code "source" is expected to be absolute. If it's not
>>      absolute we should fail. This can be achieved by passing -1/-EBADF,
>>      afaict.
>
> D'oh! Okay. I hadn't considered that use case for an invalid dirfd.
> (And now I've done some adjustments to openat(2),which contains a
> rationale for the *at() functions.)
>
> So, now I understand your purpose, but still the code is obscure,
> since
>
> * You use a magic value (-EBADF) rather than (say) -1.
> * There's no explanation (comment about) of the fact that you want
>   to prevent relative pathnames.
>
> So, I've changed the code to use -1, not -EBADF, and I've added some
> comments to explain that the intent is to prevent relative pathnames.
> Okay?

Sounds good.

>
> But, there is still the meta question: what's the problem with using
> a relative pathname?

Nothing per se. Ok, you asked so it's your fault:
When writing programs I like to never use relative paths with AT_FDCWD
because. Because making assumptions about the current working directory
of the calling process is just too easy to get wrong; especially when
pivot_root() or chroot() are in play.
My absolut preference (joke intended) is to open a well-known starting
point with an absolute path to get a dirfd and then scope all future
operations beneath that dirfd. This already works with old-style
openat() and _very_ cautious programming but openat2() and its
resolve-flag space have made this **chef's kiss**.
If I can't operate based on a well-known dirfd I use absolute paths with
a -EBADF dirfd passed to *at() functions.

Signed-off-by: Michael Kerrisk <mtk.manpages@gmail.com>
This commit is contained in:
Michael Kerrisk 2021-08-13 00:26:37 +02:00
parent 669d6302cb
commit 4c313d979d
1 changed files with 12 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
man2/mount_setattr.2
View File

@ -981,6 +981,18 @@ main(int argc, char *argv[])
/* In the following, \-1 as the \(aqdirfd\(aq argument ensures that
open_tree() fails if \(aqsource\(aq is not an absolute pathname. */
.\" Christian Brauner
.\" When writing programs I like to never use relative paths with AT_FDCWD
.\" because. Because making assumptions about the current working directory
.\" of the calling process is just too easy to get wrong; especially when
.\" pivot_root() or chroot() are in play.
.\" My absolut preference (joke intended) is to open a well-known starting
.\" point with an absolute path to get a dirfd and then scope all future
.\" operations beneath that dirfd. This already works with old-style
.\" openat() and _very_ cautious programming but openat2() and its
.\" resolve-flag space have made this **chef's kiss**.
.\" If I can't operate based on a well-known dirfd I use absolute paths
.\" with a -EBADF dirfd passed to *at() functions.
int fd_tree = open_tree(\-1, source,
OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC |