Re: [PATCH v5 15/23] lib: rspdm: Support SPDM get_capabilities

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rspdm: swapped fixed and extended alg_count nibbles in from_bytes()
--

commit 9a97b825dcf3405e60a1e8252ec1b1e885dfcb08
Author: Alistair Francis <alistair.francis@wdc.com>

lib: rspdm: Support SPDM get_capabilities

Add support for the SPDM GET_CAPABILITIES command and response validation,
including parsing the SPDM 1.3+ SupportedAlgorithms block.

> diff --git a/lib/rspdm/validator.rs b/lib/rspdm/validator.rs
> index 98922638a228b..080c2f2c49376 100644
> --- a/lib/rspdm/validator.rs
> +++ b/lib/rspdm/validator.rs

[ ... ]

> @@ -176,3 +184,312 @@ fn validate(unvalidated: KVec<u8>, context: &'c SpdmState<'c, T>) -> Result<Self
>          })
>      }
>  }

[ ... ]

> +impl SupportedAlgorithms {
> +    #[allow(clippy::get_first)]
> +    fn from_bytes(buf: &[u8]) -> Result<Self, Error> {

[ ... ]

> +        let mut alg_struct = KVec::new();
> +        for _ in 0..alg_struct_count {
> +            let alg_type = *buf.get(offset).ok_or(EPROTO)?;
> +            let alg_count = *buf.get(offset + 1).ok_or(EPROTO)?;
> +
> +            let fixed_alg_count = (alg_count & 0xf) as usize;
> +            let ext_alg_count = (alg_count >> 4) as usize;

[Severity: High]
Are the high and low nibbles of alg_count swapped here in
SupportedAlgorithms::from_bytes()?

Per DSP0274, the AlgCount field in an AlgStructure entry encodes
FixedAlgCount in bits [7:4] (alg_count >> 4) and ExtAlgCount in bits [3:0]
(alg_count & 0xf).

When an SPDM 1.3+ responder sets SPDM_CAP_SUPPORTED_ALGORITHMS in
header.param1 and includes AlgStructure entries in the CAPABILITIES
response, spdm_authenticate() -> SpdmState::get_capabilities() ->
GetCapabilitiesRsp::validate() -> SupportedAlgorithms::from_bytes() will
compute the wrong sizes for alg_supported and alg_external (for example,
treating a standard alg_count of 0x20 as 0 fixed bytes and 2 extended
32-bit algorithms instead of 2 fixed bytes and 0 extended algorithms),
causing offset validation to fail with EPROTO.

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