linux-kernel - Re: [PATCH] x86, mpx: Ensure unused arguments of prctl() MPX requests are 0

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:	Sat, 10 Jan 2015 19:39:11 +0100
From:	Andi Kleen <andi@...stfloor.org>
To:	"Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>
Cc:	Dave Hansen <dave.hansen@...el.com>,
	Andi Kleen <andi@...stfloor.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	Qiaowei Ren <qiaowei.ren@...el.com>,
	lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86, mpx: Ensure unused arguments of prctl() MPX
 requests are 0

On Sat, Jan 10, 2015 at 02:49:07PM +0100, Michael Kerrisk (man-pages) wrote:
> On 01/09/2015 07:34 PM, Dave Hansen wrote:
> > On 01/09/2015 10:25 AM, Michael Kerrisk (man-pages) wrote:
> >> On 9 January 2015 at 18:25, Andi Kleen <andi@...stfloor.org> wrote:
> >>> "Michael Kerrisk (man-pages)" <mtk.manpages@...il.com> writes:
> >>>> From: Michael Kerrisk <mtk.manpages@...il.com>
> >>>>
> >>>> commit fe8c7f5cbf91124987106faa3bdf0c8b955c4cf7 added two new prctl()
> >>>> operations, PR_MPX_ENABLE_MANAGEMENT and PR_MPX_DISABLE_MANAGEMENT.
> >>>> However, no checks were included to ensure that unused arguments
> >>>> are zero, as is done in many existing prctl()s and as should be
> >>>> done for all new prctl()s. This patch adds the required checks.
> >>>
> >>> This will break the existing gcc run time, which doesn't zero these
> >>> arguments.
> >>
> >> I'm a little lost here. Weren't these flags new in the
> >> as-yet-unreleased 3.19? How does gcc run-time depends on them already?
> > 
> > These prctl()s have been around in some form or another for a few months
> > since the patches had not yet been merged in to the kernel.  There is
> > support for them in a set of (yet unmerged) gcc patches, as well as some
> > tests which are only internal to Intel.
> > 
> > This change will, indeed, break those internal tests as well as the gcc
> > patches.  As far as I know, the code is not in production anywhere and
> > can be changed.  The prctl() numbers have changed while the patches were
> > out of tree and it's a somewhat painful process each time it changes.
> > It's not impossible, just painful.
> 
> So, sounds like thinks can be fixed (with mild inconvenience), and they
> should be fixed before 3.19 is actually released.

FWIW I added these checks to prctl first, but in hindsight it was a
mistake.

The glibc prctl() function is stdarg. Often you only have a single
extra argument, so you need to add 4 zeroes. There is no compile
time checking. It is very easy to get wrong and miscount the zeroes, 
happened several times.  The failure may be hard to catch, because
it only happens at runtime.

Also the extra zeroes look ugly in the source.

And it doesn't really buy you anything because it's very cheap
to add new prctl numbers if you want to extend something.

So I would advise against it.

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