[{"id":3688154,"web_url":"http://patchwork.ozlabs.org/comment/3688154/","msgid":"<20260508042327.C08FAC2BCB4@smtp.kernel.org>","list_archive_url":null,"date":"2026-05-08T04:23:27","subject":"Re: [PATCH 15/18] lib: rspdm: Support SPDM get_certificate","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] Unbounded lifetime in `Validate` implementation subverts borrow checker, causing strict aliasing UB.\n- [High] Improper vector length accumulation via `inc_len` causes capacity overflow and memory corruption.\n- [High] Creating an uninitialized memory slice causes Undefined Behavior.\n- [High] Out-of-bounds memory reference creation before verifying `certs_buf` length.\n- [High] Missing little-endian conversions for SPDM fields break the protocol on Big Endian architectures.\n- [High] Raw `u16` arithmetic on untrusted network lengths can overflow and panic the kernel.\n- [Medium] The validation of the certificate response size incorrectly checks against the request struct.\n--\n\ncommit 2a3b232f8fe8f4917dbcbbbbc545cdee33becabb\nAuthor: Alistair Francis \n\nlib: rspdm: Support SPDM get_certificate\n\nThis commit adds support for the SPDM GET_CERTIFICATE command. It introduces\nthe get_certificate function and related state exchange logic to retrieve\nand validate certificate chains.\n\n> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs\n> index bcb1cc955c4c6..69b6f67a6ef52 100644\n> --- a/lib/rspdm/state.rs\n> +++ b/lib/rspdm/state.rs\n[ ... ]\n> @@ -661,4 +671,119 @@ pub(crate) fn get_digests(&mut self) -> Result<(), Error> {\n> \n> Ok(())\n> }\n> +\n> + fn get_cert_exchange(\n> + &mut self,\n> + request_buf: &mut [u8],\n> + response_vec: &mut KVec,\n> + rsp_sz: usize,\n> + ) -> Result<&mut GetCertificateRsp, Error> {\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 this create a slice over uninitialized memory? Since response_vec was\nallocated with KVec::with_capacity(rsp_sz, GFP_KERNEL), it hasn't been\ninitialized to valid u8 values.\n\n> +\n> + let rc = self.spdm_exchange(request_buf, response_buf)?;\n> +\n> + if rc < (core::mem::size_of::() as i32) {\n\nIs it correct to validate the response size against GetCertificateReq here?\n\nShould this check against GetCertificateRsp instead, in case the structures\ndiverge in size in the future?\n\n> + pr_err!(\"Truncated certificate 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\nSince get_certificate() reuses response_vec in a while loop across multiple\nexchanges without clearing it, does calling inc_len() here continuously\naccumulate the length?\n\nIf spdm_exchange always writes to the start of the buffer, could this\naccumulation eventually exceed the vector's allocated capacity?\n\n[ ... ]\n> + pub(crate) fn get_certificate(&mut self, slot: u8) -> Result<(), Error> {\n> + let mut request = GetCertificateReq::default();\n> + request.version = self.version;\n> + request.param1 = slot;\n> +\n> + let req_sz = core::mem::size_of::();\n> + let rsp_sz = (core::mem::size_of::() as u32 + u16::MAX as u32)\n> + .min(self.transport_sz) as usize;\n> +\n> + request.offset = 0;\n> + request.length = (rsp_sz - core::mem::size_of::()) as u16;\n\nAre we missing a conversion to little-endian for request.length here?\n\n[ ... ]\n> + let response = self.get_cert_exchange(request_buf, &mut response_vec, rsp_sz)?;\n> +\n> + let total_cert_len =\n> + ((response.portion_length + response.remainder_length) & 0xFFFF) as usize;\n\nSince portion_length and remainder_length are read directly from the\nuntrusted SPDM response, do they need to be converted from little-endian?\n\nCould this addition overflow u16 and cause a panic if the responder provides\nlarge lengths? Should they be cast to usize before adding?\n\n[ ... ]\n> + while remainder_length > 0 {\n> + request.offset = offset.to_le() as u16;\n\nDoes offset.to_le() perform a byte swap on the 64-bit usize before\ntruncating it to u16? On big-endian systems, wouldn't this shift zeroes\ninto the lower 16 bits and always send an offset of 0?\n\nShould this be (offset as u16).to_le() instead?\n\n[ ... ]\n> + if response.portion_length == 0\n> + || (response.param1 & 0xF) != slot\n> + || offset as u16 + response.portion_length + response.remainder_length\n> + != total_cert_len as u16\n\nCould this arithmetic overflow u16 if a malicious responder supplies large\nlength values?\n\n[ ... ]\n> + let header_length = core::mem::size_of::() + self.hash_len;\n> +\n> + let ptr = certs_buf.as_mut_ptr();\n> + // SAFETY: `SpdmCertChain` is repr(C) and packed, so we can convert it from a slice\n> + let ptr = ptr.cast::();\n> + // SAFETY: `ptr` came from a reference and the cast above is valid.\n> + let certs: &mut SpdmCertChain = unsafe { &mut *ptr };\n> +\n> + if total_cert_len < header_length\n> + || total_cert_len as u16 != certs.length\n> + || total_cert_len != certs_buf.len()\n> + {\n\nIs it safe to create the mutable reference to SpdmCertChain before verifying\nthe length of certs_buf?\n\nIf the responder replies with a truncated payload, could this create a\nreference to unallocated memory? Should the boundary checks happen before\ncreating the reference?\n\nAlso, does certs.length need to be converted from little-endian before\ncomparing it here?\n\n> diff --git a/lib/rspdm/validator.rs b/lib/rspdm/validator.rs\n> index 1e5ee8a7582b1..8b44a056b3354 100644\n> --- a/lib/rspdm/validator.rs\n> +++ b/lib/rspdm/validator.rs\n[ ... ]\n> +impl Validate<&mut Unvalidated>> for &mut GetCertificateRsp {\n> + type Err = Error;\n> +\n> + fn validate(unvalidated: &mut Unvalidated>) -> Result {\n> + let raw = unvalidated.raw_mut();\n> + if raw.len() < mem::size_of::() {\n> + return Err(EINVAL);\n> + }\n> +\n> + let ptr = raw.as_mut_ptr();\n> + // CAST: `GetCertificateRsp` 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 GetCertificateRsp = unsafe { &mut *ptr };\n> +\n> + Ok(rsp)\n> + }\n> +}\n\nDoes returning Self here decouple the returned reference's lifetime from the\nunvalidated parameter's lifetime?\n\nIn get_cert_exchange(), could this allow the returned response reference\nto outlive the borrow of the underlying vector, potentially causing\noverlapping mutable and immutable references to the same memory allocation?","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=PsQA/NqH;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.232.135.74; helo=sto.lore.kernel.org;\n envelope-from=linux-pci+bounces-54190-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=\"PsQA/NqH\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sto.lore.kernel.org (sto.lore.kernel.org [172.232.135.74])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4gBbb809NCz1yJq\n\tfor ; Fri, 08 May 2026 14:23:31 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sto.lore.kernel.org (Postfix) with ESMTP id 48FC13005A82\n\tfor ; Fri, 8 May 2026 04:23:29 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 2F7022DEA8F;\n\tFri, 8 May 2026 04:23:28 +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 0AF3F14F70\n\tfor ; Fri, 8 May 2026 04:23:28 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id C08FAC2BCB4;\n\tFri, 8 May 2026 04:23:27 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778214208; cv=none;\n b=n9m0gfe3wtl2y7t0wg4Pcp7YivhVg7HSX2/YxWhkFETZ1TC3Fm/eKkyKzWYIgAAineheJbr6hfW+lLNbGvu01m/+bA9X8zY3lQUJnudsn24LF2cRL6yteEsy9j20knEz/WiLUZgxTcXBn6N0HfS+mtZEAD82ti61bKwHQ66A734=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778214208; c=relaxed/simple;\n\tbh=8P9bWeonzLDF0hKTjVu0kgMjLh6yrqQACqRbY64iv9c=;\n\th=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date:\n\t Message-Id;\n b=Lq6Eblv5EbGhEBLbyQpwvBF8ozLuCZHDdywXVNIy8UjERINd+iYOO3qYJO2lTOzFoIotV1BtPTcogth5ABIrlRY/XX+Ij3z+LYMVMqgDp1eSJ9cBqpAbETjUnhBAZLTXLmGNOQkhPerkq4HBP6jHiq43A7RGmzdksYUg0d+t0/I=","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=PsQA/NqH; 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=1778214207;\n\tbh=8P9bWeonzLDF0hKTjVu0kgMjLh6yrqQACqRbY64iv9c=;\n\th=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From;\n\tb=PsQA/NqH1mtl3YetX1Q256pDZR06MpR3FfHjjsmBv1hiWHjpNnCKdnjGcKcp+9bMd\n\t 5iEmzBRKiUBRBJI1BKAio1NuveKPJvNdnOVAnPca7eRM2rhdtqkMwn0opMxpxX3wd4\n\t B58CWkUHtQtgOKepOQBwQtfMYgD56qp50+yaoWaPBI4Gu74Q+vdMDdgf0KJ9mN1+pX\n\t BW/WeSNjxi8JKq1muEUp9eEKjJ7zZCaWMvAYfK1qD1F+cla184kqPFyAfjLnmgD0+/\n\t I4K97vG1aGAZYozk1GdmMFpsqVTrCD63Qu85/n+HtuqXVYS5bqfgz3Gcoan8s3pmEw\n\t bQsjTaQ6Bnccw==","From":"sashiko-bot@kernel.org","Subject":"Re: [PATCH 15/18] lib: rspdm: Support SPDM get_certificate","Reply-To":"sashiko@lists.linux.dev","To":"alistair23@gmail.com","Cc":"linux-pci@vger.kernel.org, ojeda@kernel.org","In-Reply-To":"<20260508031710.514574-16-alistair.francis@wdc.com>","References":"<20260508031710.514574-16-alistair.francis@wdc.com>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Date":"Fri, 08 May 2026 04:23:27 +0000","Message-Id":"<20260508042327.C08FAC2BCB4@smtp.kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":""}}]