linux-kernel - Re: [PATCH 1/2] rcu: add rcu_access_pointer and rcu_dereference

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]

Date:	Wed, 07 Apr 2010 16:40:09 +0100
From:	David Howells <dhowells@...hat.com>
To:	Eric Dumazet <eric.dumazet@...il.com>, paulmck@...ux.vnet.ibm.com
Cc:	dhowells@...hat.com, Trond.Myklebust@...app.com,
	linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] rcu: add rcu_access_pointer and rcu_dereference_protect

Eric Dumazet <eric.dumazet@...il.com> wrote:

> This is not the version Paul posted. 
> 
> Removing checks just to shutup warnings ?

No.  I don't see the point in the condition.

> All the point is to get lockdep assistance, and you throw it away.
> 
> We want to explicit the condition, so that RCU users can explicitly
> state what protects their data.

You've missed the point.

For rcu_access_pointer(), _nothing_ protects the data, not only that, we don't
care: we're only checking the pointer.

For rcu_dereference_protect[ed](), I don't see that the check helps.  You
don't need to be holding the RCU lock to call it, but you do need to hold all
the requisite locks required to exclude others modifying it.  That's a
precondition for calling this function, so is there any point in testing it
again?

For instance, consider the following pseudocode:

	do_something(struct foo *p)
	{
		struct bar *b;
		spin_lock(&foo->lock);
		b = rcu_dereference_protected(
			foo->bar, lockdep_is_held(&foo->lock));
		do_something_to_bar(b);
		spin_unlock(&foo->lock);
	}

is there any need for the condition?  Does lockdep_is_held() have any side
effects beyond those listed in the Documentation directory or on its attached
banner comments?

Furthermore, I think the condition in rcu_dereference_check() may well be
misused.  For instance, Paul suggested:

	cred = rcu_dereference_check(delegation->cred,
				     delegation->inode == NULL);

but if 'c' is supposed to be the locks that protect the data, is this a valid
check?

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