linux-kernel - Re: [PATCH 3/3] kref: Remove the memory barriers

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:	Tue, 13 Dec 2011 12:51:25 +0100
From:	Oliver Neukum <oneukum@...e.de>
To:	Greg KH <greg@...ah.com>
Cc:	Ming Lei <tom.leiming@...il.com>,
	Peter Zijlstra <peterz@...radead.org>, gregkh@...e.de,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	ostrikov@...dia.com, adobriyan@...il.com, eric.dumazet@...il.com,
	mingo@...e.hu
Subject: Re: [PATCH 3/3] kref: Remove the memory barriers

Am Dienstag, 13. Dezember 2011, 00:14:19 schrieb Greg KH:
> > I guess I worried not about the increment, but the decrement.
> > Which makes me wonder what happens if you don't intend
> > to get the kref again, but need to make sure it is usually freed,
> > like:
> > 
> > CPU A                                                         CPU B
> > 
> > kref_get(p)
> > start_io(p)
> >                                                                       [interrupt from IO]
> >                                                                       kref_put(p)
> > 
> > You need an ordering primitive between start_io() and kref_get()
> > or the counter could go negative.
> 
> Really?  On an atomic variable?  I didn't think this was needed for
> atomics to ensure this type of thing couldn't happen.

If you use an atomic variable you can be sure that the result will be
mathematically correct, even if you touch the variable from many CPUs.
(with add & sub of course) That is, refering to that variable.

It does not guarantee ordering

CPU A								CPU B
atomic_set(&a, 1);
atomic_set(&b, 1);
atomic_set(&c, 1);
									while (!atomic_read(&c));
									d = atomic_read(&a) + atomic_read(&b);

is asking for trouble. You need to do:

CPU A								CPU B
atomic_set(&a, 1);
atomic_set(&b, 1);
smp_wmb();
atomic_set(&c, 1);
									while (!atomic_read(&c));
									smp_rmb();
									d = atomic_read(&a) + atomic_read(&b);

Now replace c with an interrupt and you see the problem. It definitely exists,
but my solution was quite bad. The wmb() must be in start_io() in the first example
I gave. Putting it into kref was the wrong place.

	Regards
		Oliver

PS: even in the example I first gave the result will eventually be 0.
But that is useless because the check for zero is done only in kref_put()


-- 
- - - 
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg) 
Maxfeldstraße 5                         
90409 Nürnberg 
Germany 
- - - 
--
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/