linux-kernel - Re: [git pull] vfs fixes

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]

Date:   Sun, 2 Apr 2017 17:10:08 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [git pull] vfs fixes

On Sun, Apr 2, 2017 at 4:59 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Sun, Apr 2, 2017 at 10:01 AM, Al Viro <viro@...iv.linux.org.uk> wrote:
>>         statx followup fixes, fix for a nasty corner case in path_init()
>> leaving path.dentry in RCU mode pointing to a dentry without DCACHE_RCUACCESS
>> and a fix for stack-smashing on alpha.
>
> These were apparently committed minutes before sending me the pull request.
>
> Why? What kind of testing did this all get?

Also, that RCU fix really stinks. It makes no sense. Any valid base
for actual pathname lookup will already have the bit set, so the only
issue is when people play games and use a non-path file descriptor
without a pathname. Right?

And that case shouldn't actually use RCU lookup AT ALL!

By definition such a case will  just be immediately unlazied anyway in
complete_walk(), so doing an RCU lookup on an empty path only adds
overhead, and there is no actual point in doing an RCU walk on an
empty pathname.

So I get the feeling that the proper fix would be just something like

      /* Don't RCU-lookup empty pathnames */
      if ((flags & LOOKUP_RCU) && !*s)
            flags &= ~LOOKUP_RCU;

at the very top of path_init(), which

 (a) makes the code more efficiant, since we don't do those
unnecessary games with sequence numbers and RCU locking only to un-RCU
it immedately

and

 (b) obviates the need for those DCACHE_RCUACCESS games entirely,
since anything that can actually be used as a base for pathname lookup
will already have that bit set, afaik.

So honestly, that fix to add DCACHE_RCUACCESS just looks like the
wrong thing to me.

                     Linus