linux-kernel - Re: [PATCH 2/4] soc: apple: rtkit: Implement OSLog buffers properly

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: <63c5cbfe-4751-4409-9be7-2fda21b09503@app.fastmail.com>
Date: Wed, 26 Feb 2025 17:51:40 +0100
From: "Sven Peter" <sven@...npeter.dev>
To: "Alyssa Rosenzweig" <alyssa@...enzweig.io>
Cc: "Janne Grunau" <j@...nau.net>, asahi@...ts.linux.dev,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 "Hector Martin" <marcan@...can.st>
Subject: Re: [PATCH 2/4] soc: apple: rtkit: Implement OSLog buffers properly

On Mon, Feb 24, 2025, at 19:09, Alyssa Rosenzweig wrote:
>> +	if (ep == APPLE_RTKIT_EP_OSLOG) {
>> +		buffer->size = FIELD_GET(APPLE_RTKIT_OSLOG_SIZE, msg);
>> +		buffer->iova = FIELD_GET(APPLE_RTKIT_OSLOG_IOVA, msg) << 12;
>> +	} else {
>> +		buffer->size = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_SIZE, msg) << 12;
>> +		buffer->iova = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_IOVA, msg);
>> +	}
>
> The shifts are suspiciously asymmetric. Are we really sure this is
> correct? My guess is that both size & iova for both oslog & buffer need
> to be page-aligned, so all 4 lines should be shifted, and the bit
> offsets should be adjusted in turn, and the lower 12-bits in oslog_size
> and buffer_iova are reserved. But that's just a guess.
>
> Anyway if this logic is really what we want it deserves a comment
> because it looks like a typo.

That guess can't be true for syslog since there's no change in behavior here
and the syslog endpoint has been working fine so far. This common code is
also used for other endpoints that request buffers and there haven't been
any issues there either. The size is just passed in "number of 4k chunks"
and the IOVA needs no additional fixups.

The entire reason for this commit is because this common logic just didn't
work for oslog. Our u-boot fork uses the same logic as used here [1]. We're stealing
a chunk of MTP's SRAM to make hand-off to Linux easier there. If either size or
IOVA was off by a factor 0x4000 this would've never worked in the first
place.

I'll add a comment that this is really what we want even though it looks odd.

Sven

[1] https://github.com/AsahiLinux/u-boot/blob/582f851413c3fd1fcd60d701ed54fff2e840b9cf/arch/arm/mach-apple/rtkit.c#L144