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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022-01-12, Andrey Zhadchenko <andrey.zhadchenko@xxxxxxxxxxxxx> wrote:
> 
> 
> 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@xxxxxxxxxx> 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

Yeah the issue is that openat/openat2 don't actually take AT_* flags and
all of the constants conflict. I would prefer not mixing O_ and AT_
flags in open (and I suspect Al would also prefer that).

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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux