Re: [PATCH v3 17/21] lib: rspdm: Support SPDM get_digests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Buffer under-allocation for SPDM 1.3+ GET_DIGESTS responses leading to transport failure, transcript truncation, and EIO crashes.
- [Medium] The GET_DIGESTS implementation validates the payload length but completely discards the returned certificate digests.
- [Low] An undersized/truncated GET_DIGESTS response incorrectly logs an 'Overflow' error.
--

commit 7b16689f564246befd0f0c43fa0d4515876746e6
Author: Alistair Francis <alistair@alistair23.me>

lib: rspdm: Support SPDM get_digests

This commit adds support for the GET_DIGESTS SPDM command.

> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
> index b78086c753702..131c22319b262 100644
> --- a/lib/rspdm/state.rs
> +++ b/lib/rspdm/state.rs
[ ... ]
> @@ -612,4 +630,64 @@ pub(crate) fn negotiate_algs(&mut self) -> Result<(), Error> {
>  
>          Ok(())
>      }
> +
> +    pub(crate) fn get_digests(&mut self) -> Result<(), Error> {
> +        let mut request = GetDigestsReq::default();
> +        request.version = self.version;
> +
> +        let req_sz = core::mem::size_of::<GetDigestsReq>();
> +        let rsp_sz = core::mem::size_of::<GetDigestsRsp>() + SPDM_SLOTS * self.hash_len;

[Severity: High]
Does this calculation fail to account for SPDM 1.3+ fields like KeyPairID,
CertificateInfo, and KeyUsageMask?

For SPDM 1.3+ compliant responders, up to 32 bytes of additional fields
can be returned. Because param2 is a bitmask of provisioned slots rather
than a count, any device using slots >= 4 will produce a response size far
larger than the allocated buffer, which could trigger an EIO crash.

Even for lower slots, this calculation omits the 1.3 fields from the
transcript, which could cause a permanent signature verification failure
in challenge authentication due to transcript desynchronization.

> +
> +        // 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 len = self.spdm_exchange(request_buf, response_vec.as_mut_slice())? as usize;
> +
> +        // The transport must report a length within the buffer we provided.
> +        if len > response_vec.len() {
> +            pr_err!("Overflowed digests response\n");
> +            return Err(EIO);
> +        }
> +        response_vec.truncate(len);
> +
> +        let response: &GetDigestsRsp = Untrusted::new(response_vec.as_slice()).validate()?;

[Severity: Medium]
Does this completely discard the returned certificate digests?

The SPDM specification mandates that requesters compare the hash of the
certificate chain with the digest returned in get_digests. There doesn't
seem to be any storage for these digests in SpdmState, meaning this
cryptographic binding between the get_digests response and the certificate
chain downloaded in get_certificate might never be verified.

> +
> +        if len
> +            < core::mem::size_of::<GetDigestsRsp>()
> +                + response.param2.count_ones() as usize * self.hash_len
> +        {
> +            pr_err!("Overflowed digests response\n");

[Severity: Low]
Should this error message indicate an underflow or truncation instead?

This condition triggers when the transport returns fewer bytes than expected
for the number of provisioned slots. Logging "Overflowed" might be
misleading when debugging transport truncation or packet loss.

> +            return Err(EIO);
> +        }

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