Re: [PATCH] fs/open: add new RESOLVE_EMPTY_PATH flag for openat2

[Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>]

On 1/12/22 17:53, Christian Brauner wrote:
> On Thu, Jan 13, 2022 at 01:43:31AM +1100, Aleksa Sarai wrote:
>> On 2022-01-12, Christian Brauner <christian.brauner@ubuntu.com> wrote:
>>> On Wed, Jan 12, 2022 at 12:02:17PM +0300, Andrey Zhadchenko wrote:
>>>> If you have an opened O_PATH file, currently there is no way to re-open
>>>> it with other flags with openat/openat2. As a workaround it is possible
>>>> to open it via /proc/self/fd/<X>, however
>>>> 1) You need to ensure that /proc exists
>>>> 2) You cannot use O_NOFOLLOW flag
>>>>
>>>> Both problems may look insignificant, but they are sensitive for CRIU.
>>>
>>> Not just CRIU. It's also an issue for systemd, LXD, and other users.
>>> (One old example is where we do need to sometimes stash an O_PATH fd to
>>> a /dev/pts/ptmx device and to actually perform an open on the device we
>>> reopen via /proc/<pid>/fd/<nr>.)
>>>
>>>> First of all, procfs may not be mounted in the namespace where we are
>>>> restoring the process. Secondly, if someone opens a file with O_NOFOLLOW
>>>> flag, it is exposed in /proc/pid/fdinfo/<X>. So CRIU must also open the
>>>> file with this flag during restore.
>>>>
>>>> This patch adds new constant RESOLVE_EMPTY_PATH for resolve field of
>>>> struct open_how and changes getname() call to getname_flags() to avoid
>>>> ENOENT for empty filenames.
>>>
>>>  From my perspective this makes sense and is something that would be
>>> very useful instead of having to hack around this via procfs.
>>>
>>> However, e should consider adding RESOLVE_EMPTY_PATH since we already
>>> have AT_EMPTY_PATH. If we think this is workable we should try and reuse
>>> AT_EMPTY_PATH that keeps the api consistent with linkat(), readlinkat(),
>>> execveat(), statx(), open_tree(), mount_setattr() etc.
>>>
>>> If AT_EMPTY_PATH doesn't conflict with another O_* flag one could make
>>> openat() support it too?
>>
>> I would much prefer O_EMPTYPATH, in fact I think this is what I called
>> it in my first draft ages ago. RESOLVE_ is meant to be related to
>> resolution restrictions, not changing the opening mode.
> 
> That seems okay to me too. The advantage of AT_EMPTY_PATH is that we
> don't double down on the naming confusion, imho.
Unfortunately AT_EMPTY_PATH is 0x1000 which is O_DSYNC (octal 010000).
At first I thought to add new field in struct open_how for AT_* flags.
However most of them are irrelevant, except AT_SYMLINK_NOFOLLOW, which
duplicates RESOLVE flags, and maybe AT_NO_AUTOMOUNT.
O_EMPTYPATH idea seems cool