Re: [PATCH v3 18/21] lib: rspdm: Support SPDM get_certificate

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] `request.length` is byte-swapped to little-endian but then immediately used as a native `usize` for buffer resizing, leading to incorrect sizing on big-endian architectures.
- [High] Potential integer underflow and kernel panic if `self.transport_sz` is less than the GetCertificateRsp header size (8 bytes).
- [Medium] Inconsistent masking of SPDM reserved bits in param1 causes spurious authentication failures.
--

commit 3d050ea39968a6dc6cb0a982af8f1746df8a12b6
Author: Alistair Francis <alistair@alistair23.me>

lib: rspdm: Support SPDM get_certificate
    
Support the GET_CERTIFICATE SPDM command.

The kernel will send a GET_CERTIFICATE request to the the responder and
then iterate over all of the certificates returned.

> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
> index 131c22319b262..1e8a4402e6342 100644
> --- a/lib/rspdm/state.rs
> +++ b/lib/rspdm/state.rs
[ ... ]
> @@ -690,4 +703,147 @@ pub(crate) fn get_digests(&mut self) -> Result<(), Error> {
[ ... ]
> +    pub(crate) fn get_certificate(&mut self, slot: u8) -> Result<(), Error> {
> +        let mut request = GetCertificateReq::default();
> +        request.version = self.version;
> +        request.param1 = slot;
> +
> +        let req_sz = core::mem::size_of::<GetCertificateReq>();
> +        let rsp_sz = (core::mem::size_of::<GetCertificateRsp>() as u32 + u16::MAX as u32)
> +            .min(self.transport_sz) as usize;
> +
> +        request.offset = 0;
> +        request.length = ((rsp_sz - core::mem::size_of::<GetCertificateRsp>()) as u16).to_le();

[Severity: High]
Can this subtraction underflow and cause a kernel panic?

If a malicious or misconfigured device advertises a data transfer size during
the capabilities exchange that is smaller than 8 bytes, self.transport_sz
will be smaller than core::mem::size_of::<GetCertificateRsp>(). 

This would cause rsp_sz - core::mem::size_of::<GetCertificateRsp>() to
underflow and panic.

> +
> +        // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice
> +        let request_buf = unsafe { from_raw_parts_mut(&mut request as *mut _ as *mut u8, req_sz) };
> +
> +        let mut response_vec: KVec<u8> = KVec::from_elem(0u8, rsp_sz, GFP_KERNEL)?;
> +
> +        let response = self.get_cert_exchange(request_buf, &mut response_vec)?;
> +
> +        if response.param1 != slot {

[Severity: Medium]
Will this initial slot verification erroneously reject valid responses?

Unlike the check inside the while loop below which uses
(response.param1 & 0xF) != slot, this check strictly compares param1
without masking reserved bits.

If a compliant external device sets bits 4-7 in param1 during a
certificate response, this will cause spurious authentication failures.

> +            pr_err!("Invalid slot response\n");
> +            return Err(EPROTO);
> +        }
> +
> +        let portion_length = response.portion_length;
> +        let rem_length = response.remainder_length;
[ ... ]
> +        while remainder_length > 0 {
> +            request.offset = offset.to_le();
> +            request.length =
> +                ((remainder_length.min(rsp_sz - core::mem::size_of::<GetCertificateRsp>())) as u16)

[Severity: High]
Does this underflow the same way as the first request if rsp_sz is less than
core::mem::size_of::<GetCertificateRsp>()?

> +                    .to_le();
> +
> +            let request_buf =
> +                unsafe { from_raw_parts_mut(&mut request as *mut _ as *mut u8, req_sz) };
> +
> +            response_vec.resize(
> +                request.length as usize + core::mem::size_of::<GetCertificateRsp>(),

[Severity: High]
Does this correctly resize the buffer on big-endian architectures?

Since request.length was just byte-swapped to little-endian using .to_le()
above, reading it directly as a native usize on a big-endian system will
produce an incorrectly large or small size.

This could lead to ENOMEM, out-of-bounds logic failures, or panics.

> +                0,
> +                GFP_KERNEL,
> +            )?;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901010347.2614656-1-alistair.francis@wdc.com?part=18