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]

Message-ID: <26698.1270657193@redhat.com>
Date:	Wed, 07 Apr 2010 17:19:53 +0100
From:	David Howells <dhowells@...hat.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	dhowells@...hat.com, paulmck@...ux.vnet.ibm.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:

> > You've missed the point.
> 
> You already claimed I dont understand RCU. I find this claim funny.
> 
> > For rcu_access_pointer(), _nothing_ protects the data, not only that, we
> > don't care: we're only checking the pointer.
> 
> How can you state this ?
> 
> Thats pretty simple, "always true" is a fine condition.
> 
> What's the problem with this ?

If the condition for rcu_access_pointer() is always "always true", then it's
redundant, right?  rcu_access_pointer() is for checking the pointer only, not
checking the payload that pointer might point to.  So, what condition are you
supposed to be checking?
Eric Dumazet <eric.dumazet@...il.com> wrote:

> > but if 'c' is supposed to be the locks that protect the data, is this a
> > valid check?
> 
> 'c' is not a lock. Its a condition.

Sorry, I meant the state of the relevant locking context.

To take your example:

> filter = rcu_dereference_check(sk->sk_filter,
> 			       atomic_read(&sk->sk_wmem_alloc) == 0);

what is the value of sk->sk_wmem_alloc to the lock context of sk->sk_filter?
Why would lockdep be interested in sk_wmem_alloc?

Surely, the assertion that the value of sk->sk_filter is related to
sk_wmem_alloc being 0 is independent of the need to dereference sk_filter for
RCU purposes.  So why are these being combined?

Why not:

	ASSERT(atomic_read(&sk->sk_wmem_alloc) == 0);
	filter = rcu_dereference(sk->sk_filter);

This is much clearer, and you're not combining an unrelated assertion with the
RCU dereference.

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/