linux-kernel - Re: Linux 2.6.38-rc5

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: <AANLkTi=8O1ZJZ2dWWwmcdV3G8LtrxSHWp=GgLxAYQE3A@mail.gmail.com>
Date:	Wed, 16 Feb 2011 08:33:32 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Al Viro <viro@...iv.linux.org.uk>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Linux 2.6.38-rc5

On Wed, Feb 16, 2011 at 8:19 AM, Al Viro <viro@...iv.linux.org.uk> wrote:
>
>        Let's shift that call of nameidata_dentry_drop_rcu_maybe() into both
> callers of do_follow_link() instead.  Marginally less obvious that we won't
> reach the guts of do_follow_link() in RCU mode, just as obvious that overall
> structure is ugly as hell and avoids making it even uglier by passing inode
> down there.  How about this:

Well, that just reverts to the old state, so it certainly will work.
But isn't it much nicer to try to keep the shared logic - including
the BUG_ON() - in do_follow_link().

Sure, it means that we have to pass in 'inode', but that's largely
free (just about everything passes three arguments in registers), and
we could eventually decide that we don't need the BUG_ON() any more
and then drop it.

So I don't see the point of duplicating logic just to remove the
(almost free) inode argument.

Does it make tons of conceptual sense to pass in 'inode' to
do_follow_link? No, it's clearly redundant information, which is the
whole point of the BUG_ON(). But it does allow that extra shared
sanity test, and we _could_ also then do

  -       if (!IS_ERR(cookie) && path->dentry->d_inode->i_op->put_link)
  -               path->dentry->d_inode->i_op->put_link(path->dentry,
nd, cookie);
  +       if (!IS_ERR(cookie) && inode->i_op->put_link)
  +               inode->i_op->put_link(path->dentry, nd, cookie);

since we've verified that 'inode' is 'path->dentry->d_inode', and all
of those should be stable over all the calls (in the non-RCU case,
which we are in).

I dunno. I don't care _deeply_, but I do have to say that I much liked
how you moved the

    if (nameidata_dentry_drop_rcu_maybe(nd, path->dentry))
      ..

into do_follow_link(). I think it made it clearer that do_follow_link
(and __do_follow_link()) aren't done with RCU.

But whatever.

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