linux-kernel - Re: [PATCH] cifs - check S

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAH2r5mvd9PFZ5sxC+QLNObX23qWpJkUNdQmS_aqYA++zxueT6w@mail.gmail.com>
Date:	Fri, 27 Apr 2012 22:33:09 -0500
From:	Steve French <smfrench@...il.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Ian Kent <raven@...maw.net>, Jeff Layton <jlayton@...hat.com>,
	linux-cifs@...r.kernel.org,
	Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] cifs - check S_AUTOMOUNT in revalidate

The fix makes sense, but it is fairly recent and I haven't had a
chance to try it,
so unless a new release is imminent, I would prefer to put in the next merge
request (I have at least one more fix likely as well) next week.

On Fri, Apr 27, 2012 at 9:46 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> Steve? I have a CIFS pull request from you now, and it doesn't contain
> this one. I don't know the code well enough to say one way or the
> other, so I haven't applied this. Pls Ack/Nak.
>
>                   Linus
>
> On Fri, Apr 27, 2012 at 1:18 AM, Ian Kent <raven@...maw.net> wrote:
>> When revalidating a dentry, if the inode wasn't known to be a dfs
>> entry when the dentry was instantiated, such as when created via
>> ->readdir(), the DCACHE_NEED_AUTOMOUNT flag needs to be set on the
>> dentry in ->d_revalidate().
>>
>> The false return from cifs_d_revalidate(), due to the inode now
>> being marked with the S_AUTOMOUNT flag, might not invalidate the
>> dentry if there is a concurrent unlazy path walk. This is because
>> the dentry reference count will be at least 2 in this case causing
>> d_invalidate() to return EBUSY. So the asumption that the dentry
>> will be discarded then correctly instantiated via ->lookup() might
>> not hold.
>>
>> Signed-off-by: Ian Kent <raven@...maw.net>
>> Reviewed-by: Jeff Layton <jlayton@...hat.com>
>> Cc: Steve French <smfrench@...il.com>
>> Cc: linux-cifs@...r.kernel.org
>> ---
>>
>>  fs/cifs/dir.c |   17 ++++++++++++-----
>>  1 files changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
>> index d172c8e..ec4e9a2 100644
>> --- a/fs/cifs/dir.c
>> +++ b/fs/cifs/dir.c
>> @@ -668,12 +668,19 @@ cifs_d_revalidate(struct dentry *direntry, struct nameidata *nd)
>>                        return 0;
>>                else {
>>                        /*
>> -                        * Forcibly invalidate automounting directory inodes
>> -                        * (remote DFS directories) so to have them
>> -                        * instantiated again for automount
>> +                        * If the inode wasn't known to be a dfs entry when
>> +                        * the dentry was instantiated, such as when created
>> +                        * via ->readdir(), it needs to be set now since the
>> +                        * attributes will have been updated by
>> +                        * cifs_revalidate_dentry().
>>                         */
>> -                       if (IS_AUTOMOUNT(direntry->d_inode))
>> -                               return 0;
>> +                       if (IS_AUTOMOUNT(direntry->d_inode) &&
>> +                          !(direntry->d_flags & DCACHE_NEED_AUTOMOUNT)) {
>> +                               spin_lock(&direntry->d_lock);
>> +                               direntry->d_flags |= DCACHE_NEED_AUTOMOUNT;
>> +                               spin_unlock(&direntry->d_lock);
>> +                       }
>> +
>>                        return 1;
>>                }
>>        }
>>



-- 
Thanks,

Steve
--
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/