On 8/7/24 22:54, Lukas Wunner wrote:
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.
It is against a device with libspdm in its firmware, likely to be older
than 3.1.0.
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.
Easier to hack lib/spdm/req-authenticate.c just to see how far I can get
with my device, now it is "Malformed certificate at slot 0 offset 0". It
is just a bit inconvenient that CMA is not a module and requires the
system reboot after every change.
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.
True and a bit hilarious :)
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.
Uff, I see. So it should probably be "Malformed algorithms response"
(where param1 is actually checked) than "Truncated algorithms response",
a minor detail though. Thanks for the explanation.
So the *reference* implementation wasn't conforming to the spec. :(
Thanks,
Lukas
--
Alexey
