On Mon, Jul 08, 2024 at 07:57:02PM +1000, Alexey Kardashevskiy wrote:
> > + rc = spdm_exchange(spdm_state, req, req_sz, rsp, rsp_sz);
>
> rsp_sz is 36 bytes here. And spdm_exchange() cannot return more than 36
> because this is how pci_doe() works...
>
> > + if (rc < 0)
> > + return rc;
> > +
> > + length = rc;
> > + if (length < sizeof(*rsp) ||
> > + length < sizeof(*rsp) + rsp->param1 * sizeof(*req_alg_struct)) {
> > + dev_err(spdm_state->dev, "Truncated algorithms response\n");
>
> ... but here you expect more than 36 as realistically rsp->param1 > 0.
> How was this tested and what do I miss here?
I assume you tested this patch set against a libspdm responder
and got a "Truncated algorithms response" error.
The short answer is, it's a bug in libspdm and the issue should
go away once you update libspdm to version 3.1.0 or newer.
If you need to stay at an older version, consider cherry-picking
libspdm commits 941f0ae0d24e ("libspdm_rsp_algorithms: fixup spec
conformance") and 065fb17b74c7 ("responder: negotiate algorithms
conformance").
The bug was found and fixed by Wilfred Mallawa when testing the
in-kernel SPDM implementation against libspdm:
https://github.com/l1k/linux/issues/3
https://github.com/DMTF/libspdm/pull/2341
https://github.com/DMTF/libspdm/issues/2344
https://github.com/DMTF/libspdm/pull/2353
Problem is, most SPDM-enabled products right now are based on
libspdm (the DMTF reference implementation) and thus are bug-by-bug
compatible. However such a software monoculture is dangerous and
having a from-scratch kernel implementation has already proven useful
to identify issues like this which otherwise wouldn't have been noticed.
The in-kernel SPDM implementation currently doesn't send any
ReqAlgStructs and per the spec, the responder isn't supposed to
send any RespAlgStructs which the requester didn't ask for.
Yet libspdm always sent a hardcoded array of RespAlgStructs.
So the *reference* implementation wasn't conforming to the spec. :(
Thanks,
Lukas
