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: <alpine.DEB.2.00.1105101435480.4023@router.home>
Date:	Tue, 10 May 2011 14:38:55 -0500 (CDT)
From:	Christoph Lameter <cl@...ux.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
cc:	Vegard Nossum <vegardno@....uio.no>,
	Pekka Enberg <penberg@...helsinki.fi>,
	casteyde.christian@...e.fr,
	Andrew Morton <akpm@...ux-foundation.org>,
	netdev@...r.kernel.org, bugzilla-daemon@...zilla.kernel.org,
	bugme-daemon@...zilla.kernel.org
Subject: Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized
 memory in __alloc_skb

On Tue, 10 May 2011, Eric Dumazet wrote:

> > > You have to disable IRQ _before_ even fetching 'object'
> >
> > The object pointer is being obtained from a per cpu structure and
> > not from the page. What is the problem with fetching the object pointer?
> >
> > > Or else, you can have an IRQ, allocate this object, pass to another cpu.
> >
> > If that occurs then TID is being incremented and we will restart the loop
> > getting the new object pointer from the per cpu structure. The object
> > pointer that we were considering is irrelevant.
> >
>
> Problem is not restart the loop, but avoiding accessing a non valid
> memory area.

Yes and could you please explain clearly what the problem is?

>
> > > This other cpu can free the object and unmap page right after you did
> > > the probe_kernel_address(object) (successfully), and before your cpu :
> > >
> > > p = get_freepointer(s, object); << BUG >>
> >
> > If the other cpu frees the object and unmaps the page then
> > get_freepointer_safe() can obtain an arbitrary value since the TID was
> > incremented. We will restart the loop and discard the value retrieved.
> >
>
>
>
> In current code I see :
>
> tid = c->tid;
> barrier();
> object = c->freelist;
>
> There is no guarantee c->tid is fetched before c->freelist by cpu.
>
> You need rmb() here.

Nope. This is not processor to processor concurrency. this_cpu operations
only deal with concurrency issues on the same processor. I.e. interrupts
and preemption.

> I claim all this would be far more simple disabling IRQ before fetching
> c->tid and c->freelist, in DEBUG_PAGE_ALLOC case.
>
> You would not even need to use magic probe_kernel_read()
>
>
> Why do you try so _hard_ trying to optimize this, I really wonder.
> Nobody is able to read this code anymore and prove its correct.

Optimizing? You think about this as concurrency issue between multiple
cpus. That is fundamentally wrong. This is dealing with access to per cpu
data and the concurrency issues are only with code running on the *same*
cpu.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ