Re: [PATCH 07/12] spdm: Introduce library to authenticate devices

[Lukas Wunner <lukas@wunner.de>]

On Fri, Feb 09, 2024 at 09:32:04PM +0100, Lukas Wunner wrote:
> On Tue, Oct 03, 2023 at 01:35:26PM +0300, Ilpo Järvinen wrote:
> > On Thu, 28 Sep 2023, Lukas Wunner wrote:
> > > +	spdm_state->responder_caps = le32_to_cpu(rsp->flags);
> > 
> > Earlier, unaligned accessors where used with the version_number_entries.
> > Is it intentional they're not used here (I cannot see what would be 
> > reason for this difference)?
> 
> Thanks, good catch.  Indeed this is not necessarily naturally aligned
> because the GET_CAPABILITIES request and response succeeds the
> GET_VERSION response in the same allocation.  And the GET_VERSION
> response size is a multiple of 2, but not always a multiple of 4.

Actually, scratch that.

I've realized that since all the SPDM request/response structs are
declared __packed, the alignment requirement for the struct members
becomes 1 byte and hence they're automatically accessed byte-wise on
arches which require that:

https://stackoverflow.com/questions/73152859/accessing-unaligned-struct-member-using-pointers#73154825

E.g. this line...

        req->data_transfer_size = cpu_to_le32(spdm_state->transport_sz);

...becomes this on arm 32-bit (multi_v4t_defconfig)...

        ldr        r3, [r5, #0x1c]   ; load spdm_state->transport_sz into r3
        lsr        r2, r3, lsr #8    ; right-shift r3 into r2 by 8 bits
        strb       r3, [r7, #0xc]    ; copy lowest byte from r3 into request
        strb       r2, [r7, #0xd]    ; copy next byte from r2 into request
        lsr        r2, r3, lsr #16   ; right-shift r3 into r2 by 16 bits
        lsr        r3, r3, lsr #24   ; right-shift r3 into r3 by 24 bits
        strb       r2, [r7, #0xe]    ; copy next byte from r2 into request
        strb       r3, [r7, #0xf]    ; copy next byte from r3 into request

...and it becomes this on x64_64, which has no alignment requirements:

        mov        eax, dword [r15+0x40] ; load spdm_state->transport_sz
        mov        dword [r12+0xc], eax  ; copy into request

So for __packed structs, get_unaligned_*() / put_unaligned_*() accessors
are not necessary and I will drop them when respinning.

Thanks,

Lukas