linux-kernel - Re: [PATCH] fuse: make fuse

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTikEStNgB58Q-797rQST3QW=maz9LiGFSC=A8kkc@mail.gmail.com>
Date:	Thu, 13 Jan 2011 12:49:53 +1100
From:	Nick Piggin <npiggin@...il.com>
To:	Miklos Szeredi <miklos@...redi.hu>
Cc:	linux-fsdevel@...r.kernel.org, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fuse: make fuse_permission() RCU aware

On Thu, Jan 13, 2011 at 1:26 AM, Miklos Szeredi <miklos@...redi.hu> wrote:
> On Wed, 12 Jan 2011, Nick Piggin wrote:
>> On Tue, Jan 11, 2011 at 11:14 PM, Miklos Szeredi <miklos@...redi.hu> wrote:
>> > From: Miklos Szeredi <mszeredi@...e.cz>
>> >
>> > Only bail out of fuse_permission() on IPERM_FLAG_RCU when it is
>> > actually necessary.
>>
>> Great, thanks for taking a look... How about d_revalidate?
>
> Yeah, here's the patch
>
> Do you want to collect these patches from fs maintainers, or should I
> submit to Linus directly?

I would like to have a look over them and ack them before they go to Linus,
but I think the most logical and merge friendly approach is just going via
filesystems trees.

> From: Miklos Szeredi <mszeredi@...e.cz>
> Subject: fuse: make fuse_dentry_revalidate() RCU aware
>
> Only bail out of fuse_dentry_revalidate() on LOOKUP_RCU when blocking
> is actually necessary.
>
> Signed-off-by: Miklos Szeredi <mszeredi@...e.cz>
> ---
>  fs/fuse/dir.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> Index: linux-2.6/fs/fuse/dir.c
> ===================================================================
> --- linux-2.6.orig/fs/fuse/dir.c        2011-01-12 13:06:04.000000000 +0100
> +++ linux-2.6/fs/fuse/dir.c     2011-01-12 13:07:30.000000000 +0100
> @@ -158,9 +158,6 @@ static int fuse_dentry_revalidate(struct
>  {
>        struct inode *inode;
>
> -       if (nd->flags & LOOKUP_RCU)
> -               return -ECHILD;
> -
>        inode = entry->d_inode;
>        if (inode && is_bad_inode(inode))
>                return 0;

Now it can be the case that entry->d_inode is not stable -- it can
go away or even flip between inodes in the case of concurrent
unlink/creat activity. And you may be using a different inode than
the namei path walk is using!

This isn't as scary as it sounds actually, because any such changes
do get detected and the path walk restarted in that case.

You might be OK becuse you do test for NULL (although it really
wants an ACCESS_ONCE() so it doesn't load a NULL or different
inodes, but that's quite theoretical).

But this is a little unfriendly for filesystems, and I do want to impress
the rule to not touch ->d_parent or ->d_inode in rcu walk mode
(just to avoid any surprises).

So what I have done in such cases is to update the API to provide
what the callers want. In this case, we could consider adding an
inode parameter to .d_revalidate, which callers can be sure matches
the inode used by vfs, and will not change.

Aside from that detail, the changes seem good. Thanks for looking
at it.

> @@ -177,6 +174,9 @@ static int fuse_dentry_revalidate(struct
>                if (!inode)
>                        return 0;
>
> +               if (nd->flags & LOOKUP_RCU)
> +                       return -ECHILD;
> +
>                fc = get_fuse_conn(inode);
>                req = fuse_get_req(fc);
>                if (IS_ERR(req))
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/