linux-kernel - Re: 3.12 Regression: dcache: Translating dentry into pathname without taking rename

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: <CAGr1u3spS2G8r7jCAnFZukE_zGkEGtsk0SSyTj3H39kK9E0_tQ@mail.gmail.com>
Date:	Wed, 13 Nov 2013 14:51:59 -0800
From:	Michael Marineau <michael.marineau@...eos.com>
To:	Al Viro <viro@...iv.linux.org.uk>
Cc:	Waiman Long <Waiman.Long@...com>, linux-kernel@...r.kernel.org
Subject: Re: 3.12 Regression: dcache: Translating dentry into pathname without
 taking rename_lock 232d2d60

On Wed, Nov 13, 2013 at 4:39 AM, Al Viro <viro@...iv.linux.org.uk> wrote:
> On Wed, Nov 13, 2013 at 03:34:13AM -0800, Michael Marineau wrote:
>> Greetings,
>>
>> Commit 232d2d60aa5469bb097f55728f65146bd49c1d25 causes intermittent
>> errors in /proc/*/fd/* where readlink returns "/" instead of the
>> correct path. This can be reproduced by the script below which copies
>> the kernel source directory structure while obsessively looking up
>> directory fds in proc from another process. Reverting
>> 232d2d60aa5469bb097f55728f65146bd49c1d25 after two related commits
>> 48f5ec21d9c67e881ff35343988e290ef5cf933f
>> 1812997720ab90d029548778c55d7315555e1fef fixes the issue.
>
> Looking into it...  It seems that we are getting to the end of
> prepend_path() with non-negative error and bptr == *buffer.
> What the...
>
> OK, I see what's going on.  We never reinitialize dentry, vfsmount and mnt
> if we decide to restart.  See if the following helps:
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index ae6ebb8..89f9671 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -2881,9 +2881,9 @@ static int prepend_path(const struct path *path,
>                         const struct path *root,
>                         char **buffer, int *buflen)
>  {
> -       struct dentry *dentry = path->dentry;
> -       struct vfsmount *vfsmnt = path->mnt;
> -       struct mount *mnt = real_mount(vfsmnt);
> +       struct dentry *dentry;
> +       struct vfsmount *vfsmnt;
> +       struct mount *mnt;
>         int error = 0;
>         unsigned seq = 0;
>         char *bptr;
> @@ -2893,6 +2893,9 @@ static int prepend_path(const struct path *path,
>  restart:
>         bptr = *buffer;
>         blen = *buflen;
> +       dentry = path->dentry;
> +       vfsmnt = path->mnt;
> +       mnt = real_mount(vfsmnt);
>         read_seqbegin_or_lock(&rename_lock, &seq);
>         while (dentry != root->dentry || vfsmnt != root->mnt) {
>                 struct dentry * parent;

That appears to do the trick! I've tried my test case against that
patch on both Linus' git tree (as of last night) and the 3.12 release.
I'm now running the long build job that initially stumbled across this
bug now but it looks good so far.
--
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/