linux-kernel - Re: [PATCH ] drivers/md: use proper rcu accessor

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]

Message-ID: <1416763873.17888.33.camel@edumazet-glaptop2.roam.corp.google.com>
Date:	Sun, 23 Nov 2014 09:31:13 -0800
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Mike Snitzer <snitzer@...hat.com>
Cc:	Pranith Kumar <bobby.prani@...il.com>,
	"Kirill A. Shutemov" <kirill@...temov.name>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...nel.org>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	Dipankar Sarma <dipankar@...ibm.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Josh Triplett <josh@...htriplett.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	David Howells <dhowells@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>, dvhart@...ux.intel.com,
	Frédéric Weisbecker <fweisbec@...il.com>,
	Oleg Nesterov <oleg@...hat.com>,
	device-mapper development <dm-devel@...hat.com>
Subject: Re: [PATCH ] drivers/md: use proper rcu accessor

On Sun, 2014-11-23 at 11:53 -0500, Mike Snitzer wrote:
> On Sun, Nov 23, 2014 at 11:40 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> > From: Eric Dumazet <edumazet@...gle.com>
> >
> > rcu_dereference() should be used in sections protected by rcu_read_lock.
> >
> > For writers, holding some kind of mutex or lock,
> > rcu_dereference_protected() is the way to go, adding explicit lockdep
> > bits.
> >
> > In __unbind(), although there is no mutex or lock held, we are about
> > to free the mapped device, so can use the constant '1' instead of a
> > lockdep_is_held()
> 
> That isn't true.  dm_hash_remove_all() -- which calls dm_destroy --
> holds _hash_lock.  Why leave __unbind() brittle in the face of future
> DM locking changes?
> 

Well, tell me. Before the 33423974bfc1 patch there was no protection.

If really you are about to delete an object, you have to be sure no one
is going to use it.

rcu_dereference_protected(X, 1) is how we express this thing, there is
nothing wrong here.

Fact that you hold a lock at this point is irrelevant and wont protect
the bug from happening. If you believe so, then you are wrong.


> > Reported-by: Kirill A. Shutemov <kirill@...temov.name>
> > Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> > Fixes: 33423974bfc1 ("dm: Use rcu_dereference() for accessing rcu pointer")
> > Cc: Pranith Kumar <bobby.prani@...il.com>
> 
> Hi Eric,
> 
> I'll pick this up once I get clarification for why your __unbind
> change is safe.. but it really would've helped if you cc'd
> dm-devel@...hat.com or myself directly (not a single person that you
> cc'd actively maintains DM).
> 

Hmm, my mailer complained because the mail had too many recipients
already. I did a 'reply' on the original thread.

> Hopefully these DM rcu "fixes" are finished after this.

You added a Signed-off-by on 33423974bfc1, not me.

Kirill gave the report 2 days ago and so far nobody fixed it.

I will send a v2 because other rcu_dereference() need to be changed as
well.


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