linux-kernel - Re: [PATCH v2] futex: Fix regression with read only mappings

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: <20110627210818.GC2095@BohrerMBP.rgmadvisors.com>
Date:	Mon, 27 Jun 2011 16:08:18 -0500
From:	Shawn Bohrer <sbohrer@...advisors.com>
To:	Darren Hart <dvhart@...ux.intel.com>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	eric.dumazet@...il.com, david@...advisors.com,
	linux-kernel@...r.kernel.org, zvonler@...advisors.com,
	hughd@...gle.com, tglx@...utronix.de, mingo@...e.hu
Subject: Re: [PATCH v2] futex: Fix regression with read only mappings

On Mon, Jun 27, 2011 at 01:41:12PM -0700, Darren Hart wrote:
> 
> 
> On 06/27/2011 11:15 AM, Peter Zijlstra wrote:
> > On Mon, 2011-06-27 at 11:40 -0500, Shawn Bohrer wrote:
> >>>>     if (PageAnon(page_head)) {
> >>>
> >>> This bit needs a comment too (unless I am the only one to whom this
> >> was
> >>> non-obvious), maybe:
> >>>
> >>>               
> >>>               /*
> >>>                * A read-only anonymous page implies a COW on a
> >>>                * MAP_PRIVATE mapping. There is no sane use-case
> >>>                * for this scenario, return -EFAULT to userspace.
> >>>                */
> >>
> >> Your comment is wrong.  Unfortunately the code is completly
> >> non-obvious to me as well, and I have no idea why it is there.  This
> >> little snippet came from Peter's suggested fix in:
> >>
> >> https://lkml.org/lkml/2011/6/6/368
> >>
> >> Sadly Peter's gone silent and I'm left wondering if he knew some
> >> corner case that should return -EFAULT with a RO anonymous page or if
> >> he _thought_ this was preventing RO MAP_PRIVATE mappings.  If it is
> >> the latter then this block can be removed because it does NOT do that.
> >>>> +           if (ro) {
> >>>> +                   err = -EFAULT;
> >>>> +                   goto out;
> >>>> +           }
> >>>> + 
> > 
> > Peter simply gets too much email.. anyway, the reason I put that there
> > is that a RO Anon page will never change and is thus a little pointless
> > to use for futex ops.
> >
> 
> Right, and that was the logic I was trying to document. Shawn, how is my
> comment above wrong? A read-only anonymous page but itself doesn't imply
> a COW, but it does it does in the context of this code from my reading.

All I can tell you is from my testing is a PROT_READ, MAP_PRIVATE
page isn't an anonymous page. In other words.

futex = (int *)mmap(0, sizeof(int), PROT_READ, MAP_PRIVATE, fd, 0);
rc = syscall(SYS_futex, futex, FUTEX_WAIT, val, 0, 0, 0);

Works just fine with my patch and does NOT return EFAULT.  Your
comment indicates the opposite.

--
Shawn


---------------------------------------------------------------
This email, along with any attachments, is confidential. If you 
believe you received this message in error, please contact the 
sender immediately and delete all copies of the message.  
Thank you.

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