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

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 01 Sep 2026 01:20:39 +0000
sashiko-bot@kernel.org wrote:

> 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.

Nope. Feature turned off by requester not asking for it (yet!)

> 
> > +
> > +        // 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.

This one is curious. I can't find a reference to the spec requiring that.
Digests are a short cut, I think you can always ignore them and just use the certs
themselves.  I couldn't find any text requiring the certs if we ignore the digests
but maybe it is hiding in there somewhere.