From 4c313d979de70af19a3a1b861f4990fc6fd3130d Mon Sep 17 00:00:00 2001 From: Michael Kerrisk Date: Fri, 13 Aug 2021 00:26:37 +0200 Subject: [PATCH] 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 --- man2/mount_setattr.2 | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/man2/mount_setattr.2 b/man2/mount_setattr.2 index 07c12e3f8..196e98242 100644 --- a/man2/mount_setattr.2 +++ b/man2/mount_setattr.2 @@ -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 |