linux-kernel - Re: [PATCH 1/3] vfs: merge .d_select_inode() into .d

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJfpegt6Y0MsNMpd3rm58wmRjQTr4nd3mYM-mVCMO3Ys7dqTmQ@mail.gmail.com>
Date:	Fri, 27 May 2016 10:14:34 +0200
From:	Miklos Szeredi <miklos@...redi.hu>
To:	James Bottomley <James.Bottomley@...senpartnership.com>
Cc:	Al Viro <viro@...iv.linux.org.uk>, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, David Howells <dhowells@...hat.com>
Subject: Re: [PATCH 1/3] vfs: merge .d_select_inode() into .d_real()

On Sun, May 22, 2016 at 6:39 PM, James Bottomley
<James.Bottomley@...senpartnership.com> wrote:
> On Tue, 2016-05-17 at 23:53 +0200, Miklos Szeredi wrote:
>> The two methods essentially do the same: find the real dentry/inode
>> belonging to an overlay dentry.  The difference is in the usage:
>>
>> vfs_open() uses ->d_select_inode() and expects the function to
>> perform copy-up if necessary based on the open flags argument.
>>
>> file_dentry() uses ->d_real() passing in the overlay dentry as well
>> as the underlying inode.
>>
>> vfs_rename() uses ->d_select_inode() but passes zero flags.
>> ->d_real() with a zero inode would have worked just as well here.
>>
>> This patch merges the functionality of ->d_select_inode() into
>> ->d_real() by adding an 'open_flags' argument to the latter.
>
> Actually, before you do this, I have a use case for keeping them
> separate:  The shiftfs prototype I've posted cannot use d_select_inode
> because it's shifting the uid/gids, so the inode permissions have to be
> done on the upper inode.  However, several filesystems make use of
> file_dentry() and for them, I need to supply the lower dentry, so I end
> up setting d_real but not d_select_inode.
>
> for me, then vfs_open and vfs_rename have to operate on the upper inode
> but all uses of file_dentry should return the lower dentry.

The calls in vfs_rename() deal with rename to hard-linked counterpart.
Unless you want to add lower to upper mapping for inodes, you'll need
this too.  Same goes for hard linked sockets:

https://git.kernel.org/cgit/linux/kernel/git/mszeredi/vfs.git/commit/?h=overlayfs-next&id=eb0a4a47ae89aaa0674ab3180de6a162f3be2ddf

So looks like the only thing you don't want this is for vfs_open().
We pass open flags into the merged ->d_real() function, but that may
be zero.  Could pass an internal O_OPEN flag to indicate this is open.

Thanks,
Miklos