[{"id":3688138,"web_url":"http://patchwork.ozlabs.org/comment/3688138/","msgid":"<20260508035011.C3D00C2BCB0@smtp.kernel.org>","list_archive_url":null,"date":"2026-05-08T03:50:11","subject":"Re: [PATCH 11/18] lib: rspdm: Support SPDM get_version","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 6 potential issue(s) to consider:\n- [Critical] Missing capacity bounds check before increasing vector length leads to out-of-bounds heap memory read.\n- [High] Creating a `&mut [u8]` slice from uninitialized heap memory triggers Undefined Behavior.\n- [High] Endianness bug when parsing little-endian SPDM version numbers.\n- [High] Out-of-bounds pointer provenance (UB) when accessing the `version_number_entries` flexible array member.\n- [High] Data race and mutable aliasing due to missing internal locking in `spdm_authenticate`.\n- [Medium] `SPDM_GET_VERSION_LEN` is too small to accommodate a maximum-sized valid GET_VERSION response.\n--\n\ncommit 0a244ec3f0839ba962c85556e66ffad42451e041\nAuthor: Alistair Francis \n\nlib: rspdm: Support SPDM get_version\n\nThis commit adds support for the SPDM GET_VERSION command to negotiate\na supported SPDM version and store it in the SpdmState.\n\n> diff --git a/lib/rspdm/consts.rs b/lib/rspdm/consts.rs\n> index 2feddde678858..5482a0f6cee09 100644\n> --- a/lib/rspdm/consts.rs\n> +++ b/lib/rspdm/consts.rs\n[ ... ]\n> @@ -54,3 +65,6 @@ fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {\n> Ok(())\n> }\n> }\n> +\n> +pub(crate) const SPDM_GET_VERSION: u8 = 0x84;\n> +pub(crate) const SPDM_GET_VERSION_LEN: usize = mem::size_of::() + u8::MAX as usize;\n\nIs this length large enough to accommodate a maximum-sized valid GET_VERSION\nresponse?\n\nA valid GET_VERSION response payload contains up to 255 16-bit version\nentries. The maximum possible valid response size would be 6 bytes (header\nand count) plus 510 bytes (entries), totaling 516 bytes.\n\nAllocating only 259 bytes means valid responses with more than 126 entries\nmight be truncated and rejected during length validation.\n\n\n> diff --git a/lib/rspdm/lib.rs b/lib/rspdm/lib.rs\n> index 758d43fba5cba..1cc6c33516fb2 100644\n> --- a/lib/rspdm/lib.rs\n> +++ b/lib/rspdm/lib.rs\n[ ... ]\n> @@ -105,7 +105,13 @@\n> /// Return 0 on success or a negative errno. In particular, -EPROTONOSUPPORT\n> /// indicates authentication is not supported by the device.\n> #[export]\n> -pub unsafe extern \"C\" fn spdm_authenticate(_state_ptr: *mut spdm_state) -> c_int {\n> +pub unsafe extern \"C\" fn spdm_authenticate(state_ptr: *mut spdm_state) -> c_int {\n> + let state: &mut SpdmState = unsafe { &mut (*(state_ptr as *mut SpdmState)) };\n\nCan concurrent FFI calls to this function on the same device state create\nmutable aliased Rust references?\n\nIf multiple callers invoke this without external synchronization, creating\na mutable reference directly from the pointer could trigger data races\nand undefined behavior. Does this need internal locking to serialize\nconcurrent invocations?\n\n\n> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs\n> index 18e81f24c7249..26b90942b298d 100644\n> --- a/lib/rspdm/state.rs\n> +++ b/lib/rspdm/state.rs\n[ ... ]\n> @@ -232,4 +237,60 @@ pub(crate) fn spdm_exchange(\n[ ... ]\n> + pub(crate) fn get_version(&mut self) -> Result<(), Error> {\n> + let mut request = GetVersionReq::default();\n[ ... ]\n> + let mut response_vec: KVec = KVec::with_capacity(SPDM_GET_VERSION_LEN, GFP_KERNEL)?;\n> + // SAFETY: `response_vec` is SPDM_GET_VERSION_LEN length, initialised, aligned\n> + // and won't be mutated\n> + let response_buf =\n> + unsafe { from_raw_parts_mut(response_vec.as_mut_ptr(), SPDM_GET_VERSION_LEN) };\n\nDoes creating a mutable slice from this vector trigger undefined behavior?\n\nKVec::with_capacity() allocates memory but leaves the contents uninitialized.\nForming a reference or slice over uninitialized memory for types that expect\nvalid initialization (like u8) is immediate undefined behavior in Rust.\n\nShould the buffer be explicitly initialized first, for example by using\nKVec::from_elem()?\n\n\n> + let rc = self.spdm_exchange(request_buf, response_buf)?;\n> +\n> + // SAFETY: `rc` is the length of data read, which will be smaller\n> + // than the capacity of the vector\n> + unsafe { response_vec.inc_len(rc as usize) };\n\nIs it guaranteed that rc will always be smaller than the capacity of the\nvector?\n\nThe length returned by spdm_exchange() originates from the hardware\ntransport function, which might report a payload length larger than the\nrequested buffer.\n\nIf a device returns a length larger than SPDM_GET_VERSION_LEN, this would\npush the vector's length beyond its actual allocation, potentially leading\nto an out-of-bounds heap memory read later on. Should there be a bounds\ncheck before increasing the vector length?\n\n\n> + let response: &mut GetVersionRsp = Untrusted::new_mut(&mut response_vec).validate_mut()?;\n> +\n> + let mut foundver = false;\n> + let unaligned = core::ptr::addr_of_mut!(response.version_number_entries) as *mut u16;\n\nDoes deriving this pointer from a strongly typed reference violate Rust's\nstrict aliasing rules?\n\nThe validate method returns a strongly typed &mut GetVersionRsp, which\nrestricts the memory provenance strictly to the struct's size (6 bytes,\nsince the trailing __IncompleteArrayField is a zero-sized type).\n\nUsing a raw pointer derived from this reference to read memory beyond\nthat 6-byte boundary is undefined behavior under Stacked Borrows.\n\n\n> + for i in 0..response.version_number_entry_count {\n> + // Creating a reference on a packed struct will result in\n> + // undefined behaviour, so we operate on the raw data directly\n> + let addr = unaligned.wrapping_add(i as usize);\n> + let alpha_version = (unsafe { core::ptr::read_unaligned::(addr) } & 0xF) as u8;\n> + let version = (unsafe { core::ptr::read_unaligned::(addr) } >> 8) as u8;\n\nWill this correctly parse the version on big-endian architectures?\n\nThe protocol version numbers are specified as little-endian 16-bit integers\n(__le16), but core::ptr::read_unaligned() reads the values natively.\n\nOn big-endian machines, reading a little-endian 0x1000 (SPDM version 1.0)\nnatively will incorrectly interpret the bytes backwards, yielding 0x0010.\nThis would extract a version of 0 and break the SPDM negotiation.","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=PvVWDLrt;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=104.64.211.4; helo=sin.lore.kernel.org;\n envelope-from=linux-pci+bounces-54181-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=\"PvVWDLrt\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sin.lore.kernel.org (sin.lore.kernel.org [104.64.211.4])\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 4gBZry3XX5z1yJq\n\tfor ; Fri, 08 May 2026 13:50:26 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sin.lore.kernel.org (Postfix) with ESMTP id 17E9C30055EB\n\tfor ; Fri, 8 May 2026 03:50:14 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id CA0FB31B83B;\n\tFri, 8 May 2026 03:50:12 +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 783A43195E4\n\tfor ; Fri, 8 May 2026 03:50:12 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id C3D00C2BCB0;\n\tFri, 8 May 2026 03:50:11 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778212212; cv=none;\n b=Cbwc+QEQVAECUuJy31ubviaTt9alU2Q/bWTA4+u3Mo9EAXr5JShsoleM97JXwaXXztXekhGbZqK7Xi4K/3Mp9xrC+4H4THK5iHARA9xdDwbKhha8Da4yLh5t6jkHwrtr1Nd/uFC6/zDsLT0PhVx7zWbJkFmahFL+NHAgFYyBkXQ=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778212212; c=relaxed/simple;\n\tbh=i6vTdlepB0GOnP9UCW++NTlQRWrcsBlBdIpytgzmDVI=;\n\th=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date:\n\t Message-Id;\n b=U9o9mn0XURL/3NbUYTY/6/Yf1fv405FiIYrq1J0PrII5mIU6cuUwUojVBLVQToPztAQsw2jff1JAwwtjUoCVAsfy/6qqdbE1acWBycy9ZPAb3qKEc2rkhLnn5YWQe5g8ALcsYjR5MUKM9O0PkrSV/YmFsvalg66V/d4GM87tCEI=","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=PvVWDLrt; 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=1778212211;\n\tbh=i6vTdlepB0GOnP9UCW++NTlQRWrcsBlBdIpytgzmDVI=;\n\th=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From;\n\tb=PvVWDLrtrUCWb0Q9fYrVtwfGfnGuI01nMjfOwTeYrAxsyzUKGO9pFyFtt1fhs5DIX\n\t xEEGCuFxIN/Y4OiyrrrxYtAycw+zDPFaS672x+7daoim1yV4xXy2jmmCK2GhOKQQhX\n\t A4+3ZKYhp1yT7al5sXttvl9Nf+WStMc+C0v8gGD7iY9usTKQgL/4riXGv0IRA855BW\n\t aRZ+tzXCKwgi3mgfbmgzSFE4ccOXKoGRLYgeg7O0CcI1US/SlW5pUrwQrsDWx6inHX\n\t nLE7u/4nLXE2HwYoqylOsaGZf6P6oL+mYB/DBwJN5QQZN7TcIx3ffbt5yq9mQf1hs1\n\t 1pzFe/FOL1v7A==","From":"sashiko-bot@kernel.org","Subject":"Re: [PATCH 11/18] lib: rspdm: Support SPDM get_version","Reply-To":"sashiko@lists.linux.dev","To":"alistair23@gmail.com","Cc":"ojeda@kernel.org, linux-pci@vger.kernel.org","In-Reply-To":"<20260508031710.514574-12-alistair.francis@wdc.com>","References":"<20260508031710.514574-12-alistair.francis@wdc.com>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Date":"Fri, 08 May 2026 03:50:11 +0000","Message-Id":"<20260508035011.C3D00C2BCB0@smtp.kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":""}}]