linux-kernel - Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

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: <20151210001012.GA20997@ZenIV.linux.org.uk>
Date:	Thu, 10 Dec 2015 00:10:12 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	Neil Brown <neilb@...e.com>
Subject: Re: [PATCHSET v2] ->follow_link() without dropping from RCU mode

On Wed, Dec 09, 2015 at 06:23:09PM +0000, Al Viro wrote:

> What's more, that dentry might very well have gone negative by that
> point.  Think what happens if, during the symlink traversal, we run
> into the hard "restart from scratch in non-RCU mode".  We'll need to
> do ->put_link() on everything we have in stack.  Regardless of what
> might've happened to dentries/inodes of symlinks involved - all we
> are really promised is that RCU-delayed parts of their destruction hadn't
> been entered yet.
> 
> Note, BTW, that *all* ->put_link() instances ignore the inode argument
> and I seriously considered dropping it completely.

PS: I toyed with the idea of replacing cookie a struct callback_head, to make
it really obvious what's going on - we are leaving a closure to be evaluated
once the caller is done with our return value.  That way ->put_link() as a
method would've been gone and we would've had something like
	init_callback(destructor, page_put_link, page);
instead of
	*cookie = page;
plus
	.put_link = page_put_link,
we have right now, etc.

The reason not to go that way (and not all that strong, at that) is that
we get an extra pointer in struct saved, i.e. slightly heavier nameidata
(there are two struct saved embedded into it).  That, or play silly buggers
with
	union {
		struct callback_head callback;
		struct inode *inode;
	};
in struct saved - we need to stash a pointer to inode between pick_link()
and get_link().

Hell knows, still might make sense to go there...  What we want is to set
a closure to be evaluated once the caller of ->get_link() is done with
the string ->get_link() returns.  We end up picking the function to be
called at saved->inode->i_op->put_link and store its argument in
saved->cookie.  Might as well just use the representation we use for such
stuff (rcu_head, etc.) and be done with that...
--
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/