linux-kernel - Re: [PATCH v2 2/2] firmware/arm_ffa: remove __le64_to

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: <d750f114-8a25-4c84-912e-b6fb407a5150@app.fastmail.com>
Date: Tue, 10 Dec 2024 09:45:51 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Yeoreum Yun" <yeoreum.yun@....com>
Cc: "Sudeep Holla" <sudeep.holla@....com>,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 nd@....com
Subject: Re: [PATCH v2 2/2] firmware/arm_ffa: remove __le64_to_cpu() when set uuid for
 direct msg v2

On Tue, Dec 10, 2024, at 08:36, Yeoreum Yun wrote:
>> On Mon, Dec 9, 2024, at 17:59, Sudeep Holla wrote:
>> > On Mon, Dec 09, 2024 at 04:27:14PM +0100, Arnd Bergmann wrote:
>>
>> Looking through the other functions in drivers/firmware/arm_ffa/driver.c,
>> I see that most of them just match the specification. One exception
>> is ffa_notification_info_get(), which incorrectly casts the
>> argument response arguments to an array of 'u16' values. Using
>> the correct bit shifts according to the specification would
>> make that work on big-endian and also more readable and
>> robust. Another one is __ffa_partition_info_get_regs(), which
>> does an incorrect memcpy() instead of decoding the values.
>>
> Conclusionly, Yes. But the RFC 4122 said with network byte order.
> to describe how uuid is saved.
>
> but I think the endianess to load the register is not a choice.
> because the spec says:
>
>     UUID Lo  x2  Bytes[0...7] of UUID with byte 0 in the low-order bits.
>     UUID Hi  x3  Bytes[8...15] of UUID with byte 8 in the low-order bits.
>
> this means UUID.bytes[0] should be loaded to x2.bytes[0].
>            UUID.bytes[1] should be loaded to x2,bytes[1]
>            ...

I meant they had the choice and chose to specify little-endian
64-bit word to encode the sequence of bytes of the standard
in-memory representation of UUIDs. 

> That's why other software spec (i.e tf-a) doesn't loads UUID from register
> wihtout swapping byte with endianess but just copy it.

If the uuid is transferred in memory, you obviously don't want to
swap it. If they pass it in registers across different endianess
code without specifying the byteorder in the caller, then they
would have the same bug.

> The bug is "not send UUID according to spec" in kernel side
> That's why it fails when I send message  with direct message version 2.
> So, it''s not change code unportable to portable but it fixes according
> to spec (load UUID as it is in register wihtout endianess).

Sorry, but you are not making sense here.

Sudeep, should I just cherry-pick your other fix from the pull
request and ignore this patch?

      Arnd