On Sun, May 13 2018, Al Viro wrote:
> On Sun, May 13, 2018 at 11:59:58AM -0700, Linus Torvalds wrote:
>> On Sun, May 13, 2018 at 11:56 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>>
>> > The whole reason why that thing is getting a dentry is that some
>> filesystems
>> > really want a *connected* dentry for getxattr. Sure, saner ones will be
>> > happy with disconnected dentry, but...
>>
>> Can we just add a big comment to that effect?
>>
>> Because I don't mind the complexity, but I do mind having code that _looks_
>> complex with no reason, where the natural reaction is "why is it bothering
>> being complex, when it could just do X".
>
> Point taken. How about the following variant?
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 4cafe6a19167..398d165f884e 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1568,8 +1568,15 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
> /* Called from d_instantiate or d_splice_alias. */
> dentry = dget(opt_dentry);
> } else {
> - /* Called from selinux_complete_init, try to find a dentry. */
> + /*
> + * Called from selinux_complete_init, try to find a dentry.
> + * Some filesystems really want a connected one, so try
> + * that first. We could split SECURITY_FS_USE_XATTR in
> + * two, depending upon that...
> + */
Could you say *which* file systems? That would make it easier to
understand the bigger picture.
thanks,
NeilBrown
> dentry = d_find_alias(inode);
> + if (!dentry)
> + dentry = d_find_any_alias(inode);
> }
> if (!dentry) {
> /*
> @@ -1674,14 +1681,19 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
> if ((sbsec->flags & SE_SBGENFS) && !S_ISLNK(inode->i_mode)) {
> /* We must have a dentry to determine the label on
> * procfs inodes */
> - if (opt_dentry)
> + if (opt_dentry) {
> /* Called from d_instantiate or
> * d_splice_alias. */
> dentry = dget(opt_dentry);
> - else
> + } else {
> /* Called from selinux_complete_init, try to
> - * find a dentry. */
> + * find a dentry. Some filesystems really want
> + * a connected one, so try that first.
> + */
> dentry = d_find_alias(inode);
> + if (!dentry)
> + dentry = d_find_any_alias(inode);
> + }
> /*
> * This can be hit on boot when a file is accessed
> * before the policy is loaded. When we load policy we
Attachment:
signature.asc
Description: PGP signature
