LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
To: Christian Brauner <christian.brauner@ubuntu.com>
Cc: mtk.manpages@gmail.com,
	Alejandro Colomar <alx.manpages@gmail.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	linux-man <linux-man@vger.kernel.org>,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: Questions re the new mount_setattr(2) manual page
Date: Fri, 13 Aug 2021 00:32:09 +0200	[thread overview]
Message-ID: <37a0c459-86f7-9686-4ae5-03316198d1cc@gmail.com> (raw)
In-Reply-To: <20210812090805.qkwjxnjitgaihlep@wittgenstein>

Hi Christian,

[...]

Thanks for checking the various wordinfs.

[...]

>>>>>>>           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.

Thanks for the clarification. I've noted your rationale in a 
comment in the manual page source so that future maintainers 
will not be puzzled!

Cheers,

Michael



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

  reply	other threads:[~2021-08-12 22:32 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-10  1:38 Michael Kerrisk (man-pages)
2021-08-10  7:12 ` Michael Kerrisk (man-pages)
2021-08-10 14:11   ` Christian Brauner
2021-08-10 19:30     ` Michael Kerrisk (man-pages)
2021-08-10 14:32 ` Christian Brauner
2021-08-10 21:06   ` Michael Kerrisk (man-pages)
2021-08-11 10:07     ` Christian Brauner
2021-08-12  5:36       ` Michael Kerrisk (man-pages)
2021-08-12  9:08         ` Christian Brauner
2021-08-12 22:32           ` Michael Kerrisk (man-pages) [this message]
2021-08-10 22:47 ` Michael Kerrisk (man-pages)
2021-08-11 10:40   ` Christian Brauner
2021-08-12  5:36     ` Michael Kerrisk (man-pages)
2021-08-12  8:38       ` Christian Brauner
2021-08-13  1:25         ` Michael Kerrisk (man-pages)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=37a0c459-86f7-9686-4ae5-03316198d1cc@gmail.com \
    --to=mtk.manpages@gmail.com \
    --cc=alx.manpages@gmail.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-man@vger.kernel.org \
    --subject='Re: Questions re the new mount_setattr(2) manual page' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).