[{"id":3688143,"web_url":"http://patchwork.ozlabs.org/comment/3688143/","msgid":"<20260508040533.59B12C2BCB0@smtp.kernel.org>","list_archive_url":null,"date":"2026-05-08T04:05:32","subject":"Re: [PATCH 12/18] lib: rspdm: Support SPDM get_capabilities","submitter":{"id":93311,"url":"http://patchwork.ozlabs.org/api/people/93311/","name":null,"email":"sashiko-bot@kernel.org"},"content":"Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:\n- [High] Incorrect request and response buffer sizes for SPDM 1.0 and 1.1 GET_CAPABILITIES exchanges.\n- [High] Missing little-endian to native-endian conversion for `response.data_transfer_size`.\n- [High] `GetCapabilitiesRsp::validate` unconditionally rejects valid SPDM 1.0 responses.\n- [High] Creating a mutable slice (`&mut [u8]`) from uninitialized memory causes Undefined Behavior.\n- [Medium] Vector length bounds check does not prevent out-of-bounds capacity overflow if transport returns a larger length.\n--\n\ncommit 1fee665b22607bae77947f31f9d54552e86f77d2\nAuthor: Alistair Francis \n\nlib: rspdm: Support SPDM get_capabilities\n\nThis commit adds support for the GET_CAPABILITIES SPDM command. It negotiates\nresponder capabilities and updates the SPDM state.\n\n> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs\n> index 26b90942b298d..e7119ffa9a691 100644\n> --- a/lib/rspdm/state.rs\n> +++ b/lib/rspdm/state.rs\n\n[ ... ]\n\n> @@ -293,4 +304,67 @@ pub(crate) fn get_version(&mut self) -> Result<(), Error> {\n> \n> Ok(())\n> }\n> +\n> + /// Obtain the supported capabilities from an SPDM session and store the\n> + /// information in the `SpdmState`.\n> + pub(crate) fn get_capabilities(&mut self) -> Result<(), Error> {\n> + let mut request = GetCapabilitiesReq::default();\n> + request.version = self.version;\n> +\n> + let (req_sz, rsp_sz) = match self.version {\n> + SPDM_VER_10 => (4, 8),\n> + SPDM_VER_11 => (8, 8),\n\nAre these buffer sizes correct for SPDM 1.0 and 1.1?\n\nAccording to the SPDM specification, both SPDM 1.0 and 1.1 responses include\na mandatory 4-byte flags field, which would make their true size 12 bytes.\nFurthermore, the 1.1 request is also 12 bytes.\n\nAdditionally, since GetCapabilitiesRsp::validate() enforces a minimum length\nof 12 bytes for SPDM 1.1, wouldn't an 8-byte response unconditionally fail\nvalidation with EINVAL?\n\n> + _ => {\n> + request.data_transfer_size = self.transport_sz.to_le();\n> + request.max_spdm_msg_size = request.data_transfer_size;\n> +\n> + (\n> + core::mem::size_of::(),\n> + core::mem::size_of::(),\n> + )\n> + }\n> + };\n> +\n> + // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice\n> + let request_buf = unsafe { from_raw_parts_mut(&mut request as *mut _ as *mut u8, req_sz) };\n> +\n> + let mut response_vec: KVec = KVec::with_capacity(rsp_sz, GFP_KERNEL)?;\n> + // SAFETY: `response_vec` is rsp_sz length, initialised, aligned\n> + // and won't be mutated\n> + let response_buf = unsafe { from_raw_parts_mut(response_vec.as_mut_ptr(), rsp_sz) };\n\nDoes creating a mutable slice from this vector trigger undefined behavior?\n\nThe safety comment mentions the memory is initialized, but\nKVec::with_capacity() allocates uninitialized capacity. Creating a &mut [u8]\nreference to uninitialized memory is undefined behavior in Rust.\n\nShould this use something like KVec::resize() or MaybeUninit to ensure the\nmemory is initialized before slicing?\n\n> +\n> + let rc = self.spdm_exchange(request_buf, response_buf)?;\n> +\n> + if rc < (rsp_sz as i32) {\n> + pr_err!(\"Truncated capabilities response\\n\");\n> + to_result(-(bindings::EIO as i32))?;\n> + }\n> +\n> + // SAFETY: `rc` is the length of data read, which will be smaller\n> + // or the same as the capacity of the vector\n> + unsafe { response_vec.inc_len(rc as usize) };\n\nThe code checks if rc is truncated, but what if a buggy transport driver\nreturns an rc strictly larger than rsp_sz?\n\nIf that happens, response_vec.inc_len() would force the length beyond the\nallocated capacity, leading to out-of-bounds memory accesses. Should there\nbe a check to ensure rc <= rsp_sz?\n\n> +\n> + let response: &mut GetCapabilitiesRsp =\n> + Untrusted::new_mut(&mut response_vec).validate_mut()?;\n> +\n> + self.rsp_caps = u32::from_le(response.flags);\n> + if (self.rsp_caps & SPDM_RSP_MIN_CAPS) != SPDM_RSP_MIN_CAPS {\n> + pr_err!(\n> + \"{:#x} capabilities are supported, which don't meet required {:#x}\",\n> + self.rsp_caps,\n> + SPDM_RSP_MIN_CAPS\n> + );\n> + to_result(-(bindings::EPROTONOSUPPORT as i32))?;\n> + }\n> +\n> + if self.version >= SPDM_VER_12 {\n> + if response.data_transfer_size < SPDM_MIN_DATA_TRANSFER_SIZE {\n\nIs an endianness conversion missing here?\n\nThe SPDM protocol transmits multi-byte fields in little-endian format, but\nresponse.data_transfer_size is read directly here.\n\nOn big-endian architectures, could this be read as an artificially massive\ninteger and bypass the minimum size bounds check? Should this use\nu32::from_le() like the flags field above it?\n\n> + pr_err!(\"Malformed capabilities response\\n\");\n> + to_result(-(bindings::EPROTO as i32))?;\n> + }\n> + self.transport_sz = self.transport_sz.min(response.data_transfer_size);\n> + }\n> +\n> + Ok(())\n> + }\n> }\n\n> diff --git a/lib/rspdm/validator.rs b/lib/rspdm/validator.rs\n> index 8f45bafd4d691..7dc55882c8809 100644\n> --- a/lib/rspdm/validator.rs\n> +++ b/lib/rspdm/validator.rs\n\n[ ... ]\n\n> @@ -141,3 +145,94 @@ fn validate(unvalidated: &mut Unvalidated>) -> Result\n> Ok(rsp)\n> }\n> }\n\n[ ... ]\n\n> +impl Validate<&mut Unvalidated>> for &mut GetCapabilitiesRsp {\n> + type Err = Error;\n> +\n> + fn validate(unvalidated: &mut Unvalidated>) -> Result {\n> + let raw = unvalidated.raw_mut();\n> +\n> + if raw.len() < mem::size_of::() {\n> + let version = *(raw.get(0).ok_or(ENOMEM))?;\n> + let version_1_1_len = mem::size_of::()\n> + - mem::size_of::<__IncompleteArrayField<__le16>>()\n> + - mem::size_of::()\n> + - mem::size_of::();\n> +\n> + if version == SPDM_VER_11 && raw.len() == version_1_1_len {\n> + // Version 1.1 of the spec doesn't include all of the fields\n> + // So let's extend the KVec with 0s so we can cast the\n> + // vector to GetCapabilitiesRsp\n> +\n> + for _i in version_1_1_len..mem::size_of::() {\n> + raw.push(0, GFP_KERNEL)?;\n> + }\n> + } else {\n> + return Err(EINVAL);\n> + }\n\nDoes this unintentionally reject valid SPDM 1.0 responses?\n\nIf the SPDM session negotiated SPDM 1.0, the response will carry the\nSPDM_VER_10 version byte and have a shorter response length. Because this\nstrictly checks against SPDM_VER_11, won't SPDM 1.0 responses fall through\nto the else branch and unconditionally return EINVAL?\n\n> + }\n> +\n> + let ptr = raw.as_mut_ptr();\n> + // CAST: `GetCapabilitiesRsp` only contains integers and has `repr(C)`.\n> + let ptr = ptr.cast::();\n> + // SAFETY: `ptr` came from a reference and the cast above is valid.\n> + let rsp: &mut GetCapabilitiesRsp = unsafe { &mut *ptr };\n> +\n> + Ok(rsp)\n> + }\n> +}","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pci@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=qaKXi9sU;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.234.253.10; helo=sea.lore.kernel.org;\n envelope-from=linux-pci+bounces-54185-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"qaKXi9sU\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org [172.234.253.10])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4gBbBm3DHyz1yJq\n\tfor ; Fri, 08 May 2026 14:05:52 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 6AC233016C8A\n\tfor ; Fri, 8 May 2026 04:05:35 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 040B02C3266;\n\tFri, 8 May 2026 04:05:34 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id D59883A1DB\n\tfor ; Fri, 8 May 2026 04:05:33 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 59B12C2BCB0;\n\tFri, 8 May 2026 04:05:33 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778213133; cv=none;\n b=hSs8JS7X1hy1cRzXTa9QlGz9daA9KXSupCjnlsCGOKcjNsyHStYE56wdjDo4ftt4bb9bZ/9Q+BoLWhLl5XkAj6EfF1ZbNwTPVxafD4MeVB4J9izBVivRuD8WPdVhYWCacIYnn3b0NGFi7Bp/9DWxClGWe2p7knDCfx7+PslxStU=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778213133; c=relaxed/simple;\n\tbh=RtBE1mkL0S505sPZHyXhOI1hc43dKZNYX50j6GMTbd4=;\n\th=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date:\n\t Message-Id;\n b=nFzfPDjdND+SsOFbkc1U1C+cOMPJA05tBtYSb2f4NVTNoFI1QtIfozqhyzsEeBSFNieC06sTkYJC4D2lHpjuizrFzv68MmQHGGrIGVii4Qil5m8hko/6+dYOaq07ExSTHN4OKxxC98GYpc7H+Rak7Ixc0N0KI6FH1UdZZUVUO8U=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=qaKXi9sU; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1778213133;\n\tbh=RtBE1mkL0S505sPZHyXhOI1hc43dKZNYX50j6GMTbd4=;\n\th=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From;\n\tb=qaKXi9sUV152QJ72fMZeuqeiDF2VbtWUXF4ozsct6t6H+wFmqgB737DTZlApMXLEj\n\t 9AO34/MMzgkCMPxuIRIcXts/H9gJKuLpYEWEG853V/TwaI07azd9g6iLuwkq+LC9O0\n\t aFRjiX7hmO1HHIa+l9+L2N2pv4oLtH6a0i1m566+TdOSvzPavxcxufazc0lcXc4hSB\n\t c0aeaWbfjKwTL8eosGE/4juxIcCHCMajUkD5j2ftTIGtWv499+9rAET57pj2YvB7C6\n\t 5oPUYPzU4/cJ5+4tMJaUb5yoyWkicnUT3lYw5FrfCRfEiWYX8dXc4N1Wle18CX3iCk\n\t +qDOUhX4r5ZWw==","From":"sashiko-bot@kernel.org","Subject":"Re: [PATCH 12/18] lib: rspdm: Support SPDM get_capabilities","Reply-To":"sashiko@lists.linux.dev","To":"alistair23@gmail.com","Cc":"ojeda@kernel.org, linux-pci@vger.kernel.org","In-Reply-To":"<20260508031710.514574-13-alistair.francis@wdc.com>","References":"<20260508031710.514574-13-alistair.francis@wdc.com>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Date":"Fri, 08 May 2026 04:05:32 +0000","Message-Id":"<20260508040533.59B12C2BCB0@smtp.kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":""}}]