[{"id":3688145,"web_url":"http://patchwork.ozlabs.org/comment/3688145/","msgid":"<20260508040629.6152DC2BCB0@smtp.kernel.org>","list_archive_url":null,"date":"2026-05-08T04:06:28","subject":"Re: [PATCH 14/18] lib: rspdm: Support SPDM get_digests","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 4 potential issue(s) to consider:\n- [High] Creating a mutable slice `&mut [u8]` from uninitialized memory is Undefined Behavior.\n- [High] Missing bounds check on the returned length before incrementing vector capacity.\n- [High] `spdm_authenticate` lacks the documented internal locking, allowing concurrent accesses to mutate state without synchronization.\n- [Medium] Incorrect structure size used in the response length validation.\n--\n\ncommit ff12daae7b365f533b9a93bfd428874ab0cac519\nAuthor: Alistair Francis \n\nlib: rspdm: Support SPDM get_digests\n\nThis commit introduces support for the GET_DIGESTS SPDM command. It\nadds the necessary constants, state tracking for certificate slots,\nand the get_digests() function to perform the exchange.\n\n> diff --git a/lib/rspdm/lib.rs b/lib/rspdm/lib.rs\n> index 72886a5dfd695..e42cfdd35524c 100644\n> --- a/lib/rspdm/lib.rs\n> +++ b/lib/rspdm/lib.rs\n> @@ -120,6 +120,10 @@\n> return e.to_errno() as c_int;\n> }\n> \n> + if let Err(e) = state.get_digests() {\n> + return e.to_errno() as c_int;\n> + }\n> +\n> 0\n> }\n\nDoes spdm_authenticate properly handle concurrent invocations?\n\nThe exported C wrapper spdm_authenticate casts the opaque pointer to an\nexclusive mutable reference &mut SpdmState without acquiring any lock.\nThe documentation explicitly promises to perform internal locking to serialize\nmultiple concurrent invocations.\n\nBecause SpdmState does not contain a lock and none is acquired, concurrent\ncalls from C drivers could create multiple aliasing &mut references to the\nexact same memory, causing undefined behavior and data races when mutating\nshared state like self.provisioned_slots.\n\nCould we add the necessary locking around the state object?\n\n> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs\n> index 34676744e5094..bcb1cc955c4c6 100644\n> --- a/lib/rspdm/state.rs\n> +++ b/lib/rspdm/state.rs\n\n[ ... ]\n\n> + pub(crate) fn get_digests(&mut self) -> Result<(), Error> {\n> + let mut request = GetDigestsReq::default();\n> + request.version = self.version;\n> +\n> + let req_sz = core::mem::size_of::();\n> + let rsp_sz = core::mem::size_of::() + SPDM_SLOTS * self.hash_len;\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 slice creation safe?\n\nKVec::with_capacity() allocates memory but leaves it uninitialized. The code\nthen calls from_raw_parts_mut() to create a &mut [u8] slice spanning the entire\ncapacity.\n\nIn Rust, creating a slice reference to uninitialized memory is immediate\nundefined behavior, as it violates the validity invariant of the u8 type. The\nSAFETY comment asserts the memory is initialized, but it actually isn't.\n\nCould we initialize the memory first (e.g., passing __GFP_ZERO to the\nallocator or explicitly zeroing it), or handle it using MaybeUninit?\n\n> +\n> + let len = self.spdm_exchange(request_buf, response_buf)?;\n> +\n> + if len < (core::mem::size_of::() as i32) {\n> + pr_err!(\"Truncated digests response\\n\");\n> + to_result(-(bindings::EIO as i32))?;\n> + }\n> +\n> + // SAFETY: `len` is the length of data read, which will be smaller\n> + // then the capacity of the vector\n> + unsafe { response_vec.inc_len(len as usize) };\n\nAre we missing a bounds check on the returned length before incrementing the\nvector's length?\n\nThe code retrieves len from self.spdm_exchange() and immediately passes it to\nresponse_vec.inc_len(). The safety comment assumes that len will inherently be\nsmaller than or equal to the capacity.\n\nIf an external device or transport layer returns a length greater than rsp_sz,\nthe inc_len call will set the vector's length beyond its physical allocation,\nleading to out-of-bounds memory accesses in subsequent operations.\n\n> +\n> + let response: &mut GetDigestsRsp = Untrusted::new_mut(&mut response_vec).validate_mut()?;\n> +\n> + if len\n> + < (core::mem::size_of::()\n> + + response.param2.count_ones() as usize * self.hash_len) as i32\n> + {\n\nIs the correct structure size used in this response length validation?\n\nThe code dynamically calculates the limit using size_of::(),\nwhich is the request structure rather than the response structure\nGetDigestsRsp.\n\nWhile both structures currently happen to be exactly 4 bytes in size, using\nthe request structure to validate a response payload is logically flawed. If\nthe response structure size is updated in the future, this bounds check will\nfail to properly validate the minimum length.","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=Jf7rrELk;\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-54187-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=\"Jf7rrELk\"","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 4gBbCW5qPyz1yJq\n\tfor ; Fri, 08 May 2026 14:06:31 +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 760333016CAA\n\tfor ; Fri, 8 May 2026 04:06:30 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 0C9C428725B;\n\tFri, 8 May 2026 04:06:30 +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 DE096279798\n\tfor ; Fri, 8 May 2026 04:06:29 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 6152DC2BCB0;\n\tFri, 8 May 2026 04:06:29 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778213189; cv=none;\n b=OutGxNZFIqTgbtskMaHnmMRf3/t5QP6wsqlwKreW6nvD0Mr/0Fz/sGK9sczW+iv19D4srjYrwwWt+UHG1NmjeoNrexQr61zttacVGo50oy2bdCuBVojCNPk+6ja8vjSERrWkiizJYijQi0RsWjrA5Jkx5FfWZYFw4A1aUaL2UMw=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778213189; c=relaxed/simple;\n\tbh=LBtgeuOwYkF+hHerZxTiZdfF2LiHiKKfjsOIsESR6Y0=;\n\th=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date:\n\t Message-Id;\n b=owdpXOhzfiisJ86ygXgArRc51Xiuj7ctErOFJMNum+IdFs9Ja5bmPZHiJCgiHImXDR5ysWYn06f9Jp500q5re8yHsFLba+JT5yahcD+V3/lyurUFUosH4LcPhyhbtt0HlV8gtzMthAFIo5GIvAx9qXKJP+5k++11K2AgwRPnwOM=","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=Jf7rrELk; 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=1778213189;\n\tbh=LBtgeuOwYkF+hHerZxTiZdfF2LiHiKKfjsOIsESR6Y0=;\n\th=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From;\n\tb=Jf7rrELkTqmo0WwnlQTLpl+nmhnfOVcupvcTmnbe2rrcCV+67B3b4W4F/Ar37ueKU\n\t 8jkgI8LHzULi2qqnW8RCQEI0kXX0pZNljnXa1rddHXwLFEc464OUtv8N7JrrnEZLfq\n\t K1rsQDJgnGmMDJZDdgMH6CI6gzip5y0W80cIHlLxAEPet1c7z0TPAaA/51CPdFoAjq\n\t M7LVKwPOndKoCt4aOSiB0m6+l1mZ3/czbkogD28ugtf8sry15rcwuEcAowp4RmRdyw\n\t hkhGhCvTSFqzPZr5RuYUyY4TLnraJtw1XHTfG7uxGfZnFWzrcGnyF91/v0J1VhsJDF\n\t s1M8iCziWPygg==","From":"sashiko-bot@kernel.org","Subject":"Re: [PATCH 14/18] lib: rspdm: Support SPDM get_digests","Reply-To":"sashiko@lists.linux.dev","To":"alistair23@gmail.com","Cc":"ojeda@kernel.org, linux-pci@vger.kernel.org","In-Reply-To":"<20260508031710.514574-15-alistair.francis@wdc.com>","References":"<20260508031710.514574-15-alistair.francis@wdc.com>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Date":"Fri, 08 May 2026 04:06:28 +0000","Message-Id":"<20260508040629.6152DC2BCB0@smtp.kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":""}}]