linux-kernel - Re: [tip:perf/core] perf: Fix broken union in ' struct perf_event_mmap

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.02.1309180950420.22814@pianoman.cluster.toy>
Date:	Wed, 18 Sep 2013 10:10:22 -0400 (EDT)
From:	Vince Weaver <vince@...ter.net>
To:	Adrian Hunter <adrian.hunter@...el.com>
cc:	mingo@...nel.org, hpa@...or.com, linux-kernel@...r.kernel.org,
	a.p.zijlstra@...llo.nl, tglx@...utronix.de,
	linux-tip-commits@...r.kernel.org
Subject: Re: [tip:perf/core] perf: Fix broken union in ' struct
 perf_event_mmap_page'

On Wed, 18 Sep 2013, Adrian Hunter wrote:

> On 17/09/13 23:23, Vince Weaver wrote:
> > 
> > This patch somehow breaks the perf-ABI.
> > 
> > If I take a program that reads "mmap->cap_usr_rdpmc" and compile it
> > against the new header with this change (say from 3.12-rc1)
> > and then run it on an old kernel (say 3.11) then I get "0" for
> > cap_usr_rdpmc.
> > 
> > If I take the same program and recompile against the old (without this 
> > patch) header and run it on 3.11, I get the expected "1" value.
> > 
> > So something about this changed the bit pattern in an incompatible 
> > fashion.
> 
> 
> cap_usr_time and cap_usr_rdpmc were occupying the same bit position i.e. bit 0
> 
> That means that cap_usr_time and cap_usr_rdpmc were both unreliable.

well then I have to say this patch wins today's award for "least 
useful commit message".  Why wasn't this important info there?
>From what it originally sounded like this was just some shuffling of
low-level C minutia, not a hard-to-resolve break in the perf ABI.

> Possible improvements are one or both of:
> 1. Add cap_usr_fixed to identify kernels that have the capabilities bits fixed
> 2. Swap the positions of cap_usr_time and cap_usr_rdpmc so that
> cap_usr_rdpmc remains in bit 0

Or just create two new fields and mark the old ones as obsolete somehow.

The problem with just making this change silently is now tools like PAPI 
that care about cap_usr_rdpmc will behave in hard-to-debug ways.

For example, code compiled against new headers but run with old kernels 
won't detect rdpmc properly

Code compiled against old headers but run with new kernels won't detect 
rdpmc properly.

It's true existing code could have problems if you ran on a machine with 
conflicting rdpmc/usr_time, but my tests never noticed that, presumably 
because rdpmc and usr_time were added at the same time (in 3.4), x86 
always enables rdpmc, and none of my code uses usr_time so I never see 
wrong time results.

Vince

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