Re: [PATCH v4 18/22] lib: rspdm: Support SPDM get_digests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rspdm: GET_DIGESTS buffer truncation and invalid length breaks SPDM 1.3+ authentication
--

commit 302ede91b9559c0e7a23017a89574c6ce5ec60eb
Author: Alistair Francis <alistair@alistair23.me>

lib: rspdm: Support SPDM get_digests
    
Support the get_digests Spdm command.

> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
> index 51055ea521d72..502d99eb13a66 100644
> --- a/lib/rspdm/state.rs
> +++ b/lib/rspdm/state.rs
> [ ... ]
> @@ -600,4 +618,40 @@ 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.header.version = self.version;
> +
> +        let rsp_sz = core::mem::size_of::<SpdmHeader>() + SPDM_SLOTS * self.hash_len;
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Does this allocation provide enough capacity for Spdm 1.3+ payloads?

For Spdm 1.3+, the response appends 4 bytes per provisioned slot for items
like KeyPairID, CertificateInfo, and KeyUsageMask.

If the device utilizes multiple slots, it seems the extra bytes will exceed
this capacity, causing transport errors when interacting with valid Spdm 1.3
or 1.4 responder devices.

> +
> +        let mut request_buf = request.to_bytes()?;
> +
> +        let mut response_vec: KVec<u8> = KVec::from_elem(0u8, rsp_sz, GFP_KERNEL)?;
> [ ... ]
> diff --git a/lib/rspdm/validator.rs b/lib/rspdm/validator.rs
> index 58f6a3d436a8b..c058dde314401 100644
> --- a/lib/rspdm/validator.rs
> +++ b/lib/rspdm/validator.rs
> [ ... ]
> @@ -697,3 +700,97 @@ fn validate(unvalidated: &[u8], context: &'c SpdmState<'c>) -> Result<Self, Self
> [ ... ]
> +        if context.version >= SPDM_VER_13 && (header.param2 & !header.param1 != 0) {
> +            pr_err!("Malformed digests response\n");
> +            return Err(EPROTO);
> +        }
> +
> +        Ok(GetDigestsRsp {
> +            header,
> +            digests,
> +            length: offset,
                        ^^^^^^

[Severity: High]
Does returning only the offset for the length drop the Spdm 1.3+ suffix
fields?

In GetDigestsRsp::validate, this length only covers the header and hashes,
which appears to explicitly ignore the Spdm 1.3+ suffix fields completely.

Since this truncated response length is appended to the host transcript,
could this silently corrupt the transcript computation and cause the challenge
authentication signature verification to fail?

> +        })
> +    }
> +}

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