[{"id":3688155,"web_url":"http://patchwork.ozlabs.org/comment/3688155/","msgid":"<20260508042523.10940C2BCB0@smtp.kernel.org>","list_archive_url":null,"date":"2026-05-08T04:25:22","subject":"Re: [PATCH 16/18] lib: rspdm: Support SPDM certificate validation","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 3 potential issue(s) to consider:\n- [High] validate_cert_chain overwrites `leaf_key` when validating multiple provisioned slots, corrupting state and breaking authentication.\n- [High] validate_cert_chain leaks dynamically allocated `x509_certificate` structures on all error paths.\n- [High] validate_cert_chain leaks the leaf `x509_certificate` structure upon successful validation.\n--\n\ncommit 3ec614986c351b34141bcbc9f49c6cce87d68be8\nAuthor: Alistair Francis \n\n lib: rspdm: Support SPDM certificate validation\n\nThis commit adds support for validating the SPDM certificate chain. It performs\nbasic sanity checks to ensure the chain is correct, uses supported signatures,\nand is not blacklisted in the kernel.\n\n> diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs\n> index 69b6f67a6ef52..4c8ee553bb699 100644\n> --- a/lib/rspdm/state.rs\n> +++ b/lib/rspdm/state.rs\n\n[ ... ]\n\n> @@ -786,4 +789,68 @@ pub(crate) fn get_certificate(&mut self, slot: u8) -> Result<(), Error> {\n> \n> Ok(())\n> }\n> +\n> + pub(crate) fn validate_cert_chain(&mut self, slot: u8) -> Result<(), Error> {\n\n[ ... ]\n\n> + let cert_ptr = unsafe {\n> + from_err_ptr(bindings::x509_cert_parse(\n> + &cert_chain_buf[offset..] as *const _ as *const c_void,\n> + cert_len as usize,\n> + ))?\n> + };\n> + let cert = unsafe { *cert_ptr };\n> +\n> + if cert.unsupported_sig || cert.blacklisted {\n> + pr_err!(\"Certificate was rejected\\n\");\n> + to_result(-(bindings::EKEYREJECTED as i32))?;\n> + }\n\nIs it possible this code leaks the x509_certificate structure on error paths?\n\nIf any validation step fails (such as cert.unsupported_sig or an error in\npublic_key_verify_signature), returning early with the ? operator will bypass\nthe x509_free_certificate cleanup for both the currently parsing certificate\n(cert_ptr) and the previously parsed certificate (prev_cert).\n\nSince they are stored as raw pointers, the drop semantics do not free them.\n\n> +\n> + if let Some(prev) = prev_cert {\n> + // Check against previous certificate\n> + let rc = unsafe { bindings::public_key_verify_signature((*prev).pub_, cert.sig) };\n> +\n> + if rc < 0 {\n> + pr_err!(\"Signature validation error\\n\");\n> + to_result(rc)?;\n> + }\n> + }\n\n[ ... ]\n\n> + if let Some(prev) = prev_cert {\n> + if let Some(validate) = self.validate {\n> + let rc = unsafe { validate(self.dev, slot, prev) };\n> + to_result(rc)?;\n> + }\n> +\n> + self.leaf_key = unsafe { Some((*prev).pub_) };\n\nDoes this overwrite self.leaf_key when validating multiple provisioned slots?\n\nThe caller in lib.rs iterates over all provisioned slots, calling\nvalidate_cert_chain() for each:\n\n let mut provisioned_slots = state.provisioned_slots;\n while (provisioned_slots as usize) > 0 {\n let slot = provisioned_slots.trailing_zeros() as u8;\n\n if let Err(e) = state.validate_cert_chain(slot) {\n return e.to_errno() as c_int;\n }\n\n provisioned_slots &= !(1 << slot);\n }\n\nSince self.leaf_key is a single Option, this will overwrite the previous public\nkey pointer without freeing it.\n\nIf a device has multiple provisioned slots, could signature verification later\nin challenge() fail because self.leaf_key holds the public key of the last\nprovisioned slot rather than the first?\n\nAlso, does this leak the leaf x509_certificate structure on successful\nvalidation?\n\nAt the successful completion of the certificate chain loop, the final\ncertificate remains in prev_cert. The public key is extracted into\nself.leaf_key, but bindings::x509_free_certificate(prev) is never called.\n\nWhile spdm_destroy() later frees the public key via public_key_free(), the\nx509_certificate structure itself and its internal fields appear to be leaked\npermanently.\n\n> + }\n> +\n> + Ok(())\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=qyK3bsFF;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c04:e001:36c::12fc:5321; helo=tor.lore.kernel.org;\n envelope-from=linux-pci+bounces-54191-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=\"qyK3bsFF\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from tor.lore.kernel.org (tor.lore.kernel.org\n [IPv6:2600:3c04:e001:36c::12fc:5321])\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 4gBbdP0YbLz1yCg\n\tfor ; Fri, 08 May 2026 14:25:28 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby tor.lore.kernel.org (Postfix) with ESMTP id 5FE4930358BB\n\tfor ; Fri, 8 May 2026 04:25:26 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id B6ED1317174;\n\tFri, 8 May 2026 04:25:23 +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 9349817BCA\n\tfor ; Fri, 8 May 2026 04:25:23 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 10940C2BCB0;\n\tFri, 8 May 2026 04:25:23 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778214323; cv=none;\n b=FSHNtphyA8sQkQEa1/wOaGgekU7er2+RtGLNGDYZGJK71v3d8lZnFW5rCGOOtD7S4PV7MX4ZKvVPp9Qhc+g9iUNIyLroFQLpuWMR/nalZH5KHx89eKDvFlgHxQELbdwh6NCqfFLmVi0mxGWYl+DtLkDqm93qRyKo4W3IdS6l5ew=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778214323; c=relaxed/simple;\n\tbh=5moNjiP+glR3IeuB3JfTCJYgT19DShuteoQK3bfNoio=;\n\th=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date:\n\t Message-Id;\n b=ua7EyQpOAT5uFmyU35xsoyn1tjCTtC2VB6cQ8KdWf5VohcmCvjCdtkHMjqF+F7vpr1xt1vAl6y/FS4FTRWEGr6wGkAkuxsTXlFcEZCfVafMrGtstWGizIDrIM13CqcUk8iOMn1GeFP5zdmvtq0PS5W8wiYQcIYzZmj4E1FdPlos=","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=qyK3bsFF; 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=1778214323;\n\tbh=5moNjiP+glR3IeuB3JfTCJYgT19DShuteoQK3bfNoio=;\n\th=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From;\n\tb=qyK3bsFFzueu/P2+0Pbx2Niu+tVzXkbZGHLWdVeB8Tp3oOtIBNCRhbFuWWXBDFv+0\n\t pDIGYLH9GLxrTnfHImHBjC9m8LMwIgDd93bxcm47aWg1SXDQVwmQuNws760aB4Cdni\n\t bIjCM194Wqpb6sadIDKv4NiubuFA6LZ9ACDwvnud8i2LLViJ/FR6xCR6ul0QDxfgau\n\t KS+d4ztZKJjUb2X/JuvStLYu0zv63M+S358BrjdODty5dWyHY+/bbhpEUlebpveRGS\n\t z1zW6dwxsKhXLxC+VHYZwtPBMaVLziwdnNng6UfWir3J6KXGn1nJuOUp55JmqXRSTO\n\t Hg82Oey3SQXUA==","From":"sashiko-bot@kernel.org","Subject":"Re: [PATCH 16/18] lib: rspdm: Support SPDM certificate validation","Reply-To":"sashiko@lists.linux.dev","To":"alistair23@gmail.com","Cc":"ojeda@kernel.org, linux-pci@vger.kernel.org","In-Reply-To":"<20260508031710.514574-17-alistair.francis@wdc.com>","References":"<20260508031710.514574-17-alistair.francis@wdc.com>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Date":"Fri, 08 May 2026 04:25:22 +0000","Message-Id":"<20260508042523.10940C2BCB0@smtp.kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":""}}]