linux-kernel - Re: [PATCH 04/16] vfs: do_last(): use inode variable

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]

Date:	Tue, 1 May 2012 14:06:13 +1000
From:	Nick Piggin <npiggin@...il.com>
To:	Miklos Szeredi <miklos@...redi.hu>
Cc:	viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, hch@...radead.org,
	torvalds@...ux-foundation.org, mszeredi@...e.cz
Subject: Re: [PATCH 04/16] vfs: do_last(): use inode variable

On 25 April 2012 22:44, Miklos Szeredi <miklos@...redi.hu> wrote:
> From: Miklos Szeredi <mszeredi@...e.cz>
>
> Use helper variable instead of path->dentry->d_inode before complete_walk().
> This will allow this code to be used in RCU mode.

What do you mean, allow it to be used?

>
> Signed-off-by: Miklos Szeredi <mszeredi@...e.cz>
> ---
>  fs/namei.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 46d4bf6..f21ddb3 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2360,15 +2360,16 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
>        if (error)
>                nd->flags |= LOOKUP_JUMPED;
>
> +       inode = path->dentry->d_inode;
>        error = -ENOENT;
> -       if (!path->dentry->d_inode)
> +       if (!inode)
>                goto exit_dput;
>
> -       if (path->dentry->d_inode->i_op->follow_link)
> +       if (inode->i_op->follow_link)
>                return NULL;
>
>        path_to_nameidata(path, nd);
> -       nd->inode = path->dentry->d_inode;
> +       nd->inode = inode;
>        /* Why this, you ask?  _Now_ we might have grown LOOKUP_JUMPED... */
>        error = complete_walk(nd);
>        if (error)

In rcu-walk mode, dentry->d_inode should not be accessed at all,
outside of the core lookup code that (should) have the correct
barriers and sequence locks.

That logic should not escape into here, so I'm just not sure what
you're doing here.

This code runs in rcu-walk mode, then at the very least you need
ACCESS_ONCE to load the inode, because ->d_inode can
become NULL right after you test it for NULL (but that would likely
be a bandaid, I'm just pointing out there's raft of potential
problems).
--
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/