linux-kernel - Re: [PATCH v2 8/8] perf record: Directly bail out for compat case

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: <96e5fac6-17a2-ea03-9b15-338b84321ecf@intel.com>
Date:   Mon, 7 Jun 2021 13:23:43 +0300
From:   Adrian Hunter <adrian.hunter@...el.com>
To:     Leo Yan <leo.yan@...aro.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Mike Leach <mike.leach@...aro.org>,
        Andi Kleen <ak@...ux.intel.com>,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 8/8] perf record: Directly bail out for compat case

On 2/06/21 3:38 pm, Leo Yan wrote:
> Hi Adrain,
> 
> On Wed, Jun 02, 2021 at 02:18:47PM +0300, Adrian Hunter wrote:
>> On 2/06/21 1:30 pm, Leo Yan wrote:
>>> Since the 64-bit atomicity is not promised in 32-bit perf, directly
>>> report the error and bail out for this case.
>>>
>>> Now only applies on x86_64 and Arm64 platforms.
>>>
>>> Suggested-by: Adrian Hunter <adrian.hunter@...el.com>
>>
>> Maybe we can do better for the compat case.
>>
>> We can assume the upper 32-bits change very seldom,
>> and always increase. So for the 'read' case:
>>
>> 	u64 first, second, last;
>> 	u64 mask = (u64)((u32)-1) << 32;
>>
>> 	do {
>> 		first = READ_ONCE(pc->aux_head);
>> 		rmb();
>> 		second = READ_ONCE(pc->aux_head);
>> 		rmb();
>> 		last = READ_ONCE(pc->aux_head);
>> 	} while ((first & mask) != (last & mask));
>> 	return second;
>>
>> For the write case, we can cause a fatal error only if the new
>> tail has non-zero upper 32-bits.  That gives up to 4GiB of data
>> before aborting:
>>
>> 	if (tail & mask)
>> 		return -1;
>> 	smp_mb();
>> 	WRITE_ONCE(pc->aux_tail, tail);
> 
> Seems to me, it's pointless to only support aux_head for 64-bit and
> support aux_tail for 32-bit.  I understand this can be helpful for the
> snapshot mode which only uses aux_head, but it still fails to support
> the normal case for AUX ring buffer using 64-bit head/tail.

I am not sure why you say it is pointless.  'perf record' would still be
able to capture up to 4GiB of data. Do you mean you usually capture more
than 4GiB of data?

I was thinking we would separate out the compat case:

#if BITS_PER_LONG == 32
	if (kernel_is_64_bit)
		return compat_auxtrace_mmap__[read_head/write_tail]()
#endif

So the non-compat cases would not be affected.