linux-kernel - Re: [RESEND v2 PATCH 1/2] aio, memory-hotplug: Fix confliction when migrating and accessing ring pages.

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:	Fri, 14 Mar 2014 11:14:46 -0400
From:	Benjamin LaHaise <bcrl@...ck.org>
To:	Gu Zheng <guz.fnst@...fujitsu.com>
Cc:	Tang Chen <tangchen@...fujitsu.com>, viro@...iv.linux.org.uk,
	jmoyer@...hat.com, kosaki.motohiro@...il.com,
	kosaki.motohiro@...fujitsu.com, isimatu.yasuaki@...fujitsu.com,
	linux-fsdevel@...r.kernel.org, linux-aio@...ck.org,
	linux-kernel@...r.kernel.org, miaox@...fujitsu.com
Subject: Re: [RESEND v2 PATCH 1/2] aio, memory-hotplug: Fix confliction when migrating and accessing ring pages.

Hi Gu,

On Fri, Mar 14, 2014 at 06:25:16PM +0800, Gu Zheng wrote:
> Hi Ben,
> On 03/13/2014 06:17 AM, Benjamin LaHaise wrote:
> 
> > Hello Tang,
> > 
> > On Wed, Mar 12, 2014 at 01:25:26PM +0800, Tang Chen wrote:
> > ... <snip> ...
> > 
> >>> Another spot is in
> >>> aio_read_events_ring() where head and tail are fetched from the ring 
> >>> without
> >>> any locking.  I also fear we'll be introducing new performance issues with
> >>> all the additonal spinlock bouncing, despite the fact that is only ever
> >>> needed for migration.  I'm going to continue looking into this today and
> >>> will try to send out a followup to this email later.
> >>
> >> In the beginning of aio_read_events_ring(), it reads head and tail, not 
> >> write.
> >> So even if ring pages are migrated, the contents of the pages will not 
> >> be changed.
> >> So reading it is OK, from old page or from the new page, I think.
> > 
> > Your assumption that reading it is okay is incorrect.  Since we do not have 
> > a reference on the page at that point, it is possible that the read of the 
> > page takes place after the page has been freed and allocated to another part 
> > of the kernel.  This would result in the read returning invalid information.
> 
> What about the following patch? It adds additional reference to protect the page
> avoid being freed when we reading it.
> ps.It is applied on linux-next(3-13).

I think that's even worse than the spinlock approach since we'll end up 
bouncing around the struct page's cacheline in addition to spinlock we're 
going to end up taking anyways.

		-ben
-- 
"Thought is the essence of where you are now."
--
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/