[{"id":3688144,"web_url":"http://patchwork.ozlabs.org/comment/3688144/","msgid":"<20260508040551.48C9CC2BCB0@smtp.kernel.org>","list_archive_url":null,"date":"2026-05-08T04:05:50","subject":"Re: [PATCH 13/18] lib: rspdm: Support SPDM negotiate_algorithms","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 7 potential issue(s) to consider:\n- [Critical] Use-After-Free and Double Free of `shash_desc` allocation.\n- [High] Invalid pointer dereference and kernel panic when `crypto_alloc_shash` fails.\n- [High] Logic error in measurement capability check bypasses algorithm validation.\n- [High] Missing upper-bound check on `rc` allows vector length to exceed capacity.\n- [High] Endianness bug in `request.length` field generation.\n- [High] Creating `&mut [u8]` slices over uninitialized memory is Undefined Behavior.\n- [High] Memory leak of the `SpdmState` struct in `spdm_destroy()`.\n--\n\ncommit 76d36cb1cc14535a6da5b5475b1186acce60ab8a\nAuthor: Alistair Francis \n\nlib: rspdm: Support SPDM negotiate_algorithms\n\nThis commit adds support for the NEGOTIATE_ALGORITHMS SPDM command.\n\n> diff --git a/lib/rspdm/lib.rs b/lib/rspdm/lib.rs\n> index 9628f258854c6..72886a5dfd695 100644\n> --- a/lib/rspdm/lib.rs\n> +++ b/lib/rspdm/lib.rs\n[ ... ]\n> @@ -123,4 +127,16 @@\n> ///\n> /// @spdm_state: SPDM session state\n> #[export]\n> -pub unsafe extern \"C\" fn spdm_destroy(_state_ptr: *mut spdm_state) {}\n> +pub unsafe extern \"C\" fn spdm_destroy(state_ptr: *mut spdm_state) {\n> + let state: &mut SpdmState = unsafe { &mut (*(state_ptr as *mut SpdmState)) };\n> +\n> + if let Some(desc) = &mut state.desc {\n> + unsafe {\n> + bindings::kfree(*desc as *mut _ as *mut c_void);\n> + }\n> + }\n> +\n\nDoes this leak the SpdmState allocation itself? The internal fields are cleaned\nup, but state_ptr itself does not appear to be freed here.\n\n> + unsafe {\n> + bindings::crypto_free_shash(state.shash);\n> + }\n> +}\n\nIf crypto_alloc_shash() fails earlier, it can return an ERR_PTR. If that\nhappens, state.shash holds an error pointer. Since crypto_free_shash()\nonly checks for NULL, will this cause a kernel panic when trying to free\nan ERR_PTR here?\n\n> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs\n> index e7119ffa9a691..34676744e5094 100644\n> --- a/lib/rspdm/state.rs\n> +++ b/lib/rspdm/state.rs\n[ ... ]\n> @@ -367,4 +427,155 @@ pub(crate) fn get_capabilities(&mut self) -> Result<(), Error> {\n[ ... ]\n> + let mut desc_vec: KVec = KVec::with_capacity(desc_len, GFP_KERNEL)?;\n> + // SAFETY: `desc_vec` is `desc_len` long\n> + let desc_buf = unsafe { from_raw_parts_mut(desc_vec.as_mut_ptr(), desc_len) };\n\nIs this creating a reference to uninitialized memory? KVec::with_capacity()\nleaves the underlying memory uninitialized, and forming a slice over\nuninitialized memory is undefined behavior in Rust.\n\n> +\n> + let desc = unsafe {\n> + core::mem::transmute::<*mut c_void, &mut bindings::shash_desc>(\n> + desc_buf.as_mut_ptr() as *mut c_void\n> + )\n> + };\n> + desc.tfm = self.shash;\n> +\n> + self.desc = Some(desc);\n\nCan this cause a use-after-free and a later double-free? desc_vec is a local\nvariable that will be dropped at the end of this function, freeing its\nbacking buffer. self.desc then holds a dangling reference. Furthermore,\nwhen spdm_destroy() cleans up the session, it calls bindings::kfree() on\nthis dangling pointer, which seems like it would corrupt the kernel\nallocator.\n\n[ ... ]\n> + pub(crate) fn negotiate_algs(&mut self) -> Result<(), Error> {\n> + let mut request = NegotiateAlgsReq::default();\n> + request.version = self.version;\n> +\n> + if self.version >= SPDM_VER_12 && (self.rsp_caps & SPDM_KEY_EX_CAP) == SPDM_KEY_EX_CAP {\n> + request.other_params_support = SPDM_OPAQUE_DATA_FMT_GENERAL;\n> + }\n> +\n> + let req_sz = core::mem::size_of::();\n> + let rsp_sz = core::mem::size_of::();\n> +\n> + request.length = req_sz as u16;\n\nThe SPDM protocol requires multi-byte fields to be transmitted in\nlittle-endian format. Will this send a corrupted, big-endian length value\non big-endian architectures? The base_asym_algo field uses to_le(), so\nperhaps this should as well.\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\nIs this also creating a reference to uninitialized memory? The safety\ncomment notes it is initialized, but KVec::with_capacity() does not initialize\nthe allocated memory.\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> + // then the capacity of the vector\n> + unsafe { response_vec.inc_len(rc as usize) };\n\nDoes this code verify that rc is not larger than rsp_sz? If\nspdm_exchange() returns a value greater than rsp_sz (for example, if a\ndevice sends extra payload data), would this set the vector length beyond\nits allocated capacity and lead to out-of-bounds memory accesses?\n\n[ ... ]\n> + if self.rsp_caps & SPDM_MEAS_CAP_MASK == SPDM_MEAS_CAP_MASK\n> + && (self.meas_hash_alg.count_ones() != 1\n> + || response.measurement_specification_sel != SPDM_MEAS_SPEC_DMTF)\n> + {\n> + pr_err!(\"Malformed algorithms response\\n\");\n> + to_result(-(bindings::EPROTO as i32))?;\n> + }\n\nDoes this condition correctly check for measurement support?\nSPDM_MEAS_CAP_MASK is defined as bits 3 and 4 (11b). According to the SPDM\nspecification, 11b is a reserved value, while valid measurement support is\n01b or 10b. By checking for strict equality with the mask, it looks like\nthis evaluates to false for legitimate devices and bypasses the algorithm\nvalidation.","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=cf57lBvC;\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-54186-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=\"cf57lBvC\"","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 4gBbBr12b0z1yJq\n\tfor ; Fri, 08 May 2026 14:05:56 +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 912A93016EE9\n\tfor ; Fri, 8 May 2026 04:05:52 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 048B528725B;\n\tFri, 8 May 2026 04:05:52 +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 D5E5C282F3B\n\tfor ; Fri, 8 May 2026 04:05:51 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 48C9CC2BCB0;\n\tFri, 8 May 2026 04:05:51 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778213151; cv=none;\n b=ilfPbviiXPQMvwBiSwHPneiyW4DjZvJV2k0hg90pKF1mFksJkYbrxXoiqCsu0Eu+RQK6mpG/1u27sRlLhviCOcR+hqSp3aSFfw9/gD1k6ko4cxu8YgO3NTwS6/biCz/WWT7h3duB8+NPtOfOnjneSbe/f8T32V3OJN4FpmQFmDY=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778213151; c=relaxed/simple;\n\tbh=IAeCcF1GVm8S7rdxSqg1+CWYUJdU/7YXnwP+a7fHzeA=;\n\th=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date:\n\t Message-Id;\n b=EL/SUoahQkWn/cE5Da+bltPmvMNCpvaHhDyU4cw17y+kjy0oYluA2UzB6dG5GSmm+rurT7BZSGLJhExFXt3Asch/c08CJDiiLU5DJ6tTGiDtEKM3Sq5VgcvzRnGN82XktP+9YEQHNWZjgS/aVlfuU6pgb43GP/xlW3VUvwgu4Gs=","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=cf57lBvC; 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=1778213151;\n\tbh=IAeCcF1GVm8S7rdxSqg1+CWYUJdU/7YXnwP+a7fHzeA=;\n\th=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From;\n\tb=cf57lBvCS9xIGmLoTbohD6NfnCIw+lx4Xyl/PtJb43g5mJczhEdEG0ndvMAKLkEwu\n\t 0Me5GyxdzAtqlbnAikR1933qmoBKUQ3OHXw14pMUOpdXXPtrrOyJR//RCSYIWSa8/C\n\t guvkXot3dO5ifTEHeT+0Q4Ndqk/ymArAt0ODYddP0idL1SZDZTSR475J2EaGMoaiEj\n\t lZBpIEnlMvT+CadpzSXX9ae/MLtagzIrqobasEXWxRoH3DeglZnmD13jWJgCuMwcti\n\t o8u+CfW7IGN61ILx/psHUl5oCuoW/xeDWE4JLuGy7yuFX0eHJ1AfzH8pgM1KMVI55G\n\t i56LaMohpOZ9A==","From":"sashiko-bot@kernel.org","Subject":"Re: [PATCH 13/18] lib: rspdm: Support SPDM negotiate_algorithms","Reply-To":"sashiko@lists.linux.dev","To":"alistair23@gmail.com","Cc":"ojeda@kernel.org, linux-pci@vger.kernel.org","In-Reply-To":"<20260508031710.514574-14-alistair.francis@wdc.com>","References":"<20260508031710.514574-14-alistair.francis@wdc.com>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Date":"Fri, 08 May 2026 04:05:50 +0000","Message-Id":"<20260508040551.48C9CC2BCB0@smtp.kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":""}}]