[{"id":3688133,"web_url":"http://patchwork.ozlabs.org/comment/3688133/","msgid":"<20260508033917.B5873C2BCB0@smtp.kernel.org>","list_archive_url":null,"date":"2026-05-08T03:39:17","subject":"Re: [PATCH 04/18] X.509: Move certificate length retrieval into new\n helper","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 1 potential issue(s) to consider:\n- [High] The exported helper `x509_get_certificate_length()` uses a flawed logical condition (`&&` instead of `||`) that fails to properly validate ASN.1 headers, leading to parser differentials, incorrect length calculations, and functional failures.\n--\n\ncommit 0f604cd0b6dd899bd029f10862a9b7010589e559\nAuthor: Lukas Wunner <lukas@wunner.de>\n\nX.509: Move certificate length retrieval into new helper\n\nThis commit extracts the certificate length parsing logic from\nx509_load_certificate_list() into a newly exported helper,\nx509_get_certificate_length(), to be reused by the upcoming in-kernel\nSPDM library.\n\n> diff --git a/crypto/asymmetric_keys/x509_loader.c b/crypto/asymmetric_keys/x509_loader.c\n> index a417413269989..25ff027fad1d3 100644\n> --- a/crypto/asymmetric_keys/x509_loader.c\n> +++ b/crypto/asymmetric_keys/x509_loader.c\n> @@ -4,28 +4,42 @@\n>  #include <linux/key.h>\n>  #include <keys/asymmetric-type.h>\n>  \n> +ssize_t x509_get_certificate_length(const u8 *p, unsigned long buflen)\n> +{\n> +\tssize_t plen;\n> +\n> +\t/* Each cert begins with an ASN.1 SEQUENCE tag and must be more\n> +\t * than 256 bytes in size.\n> +\t */\n> +\tif (buflen < 4)\n> +\t\treturn -EINVAL;\n> +\n> +\tif (p[0] != 0x30 &&\n> +\t    p[1] != 0x82)\n> +\t\treturn -EINVAL;\n\nDoes this logical condition properly enforce the expected ASN.1 header?\n\nIf an untrusted device supplies a valid certificate smaller than 256 bytes\nusing a 1-byte length encoding (for example p[0] = 0x30, p[1] = 0x81), this\ncheck evaluates to false because p[0] is equal to 0x30. This allows the\nshort certificate to bypass the check.\n\n> +\n> +\tplen = (p[2] << 8) | p[3];\n> +\tplen += 4;\n\nIf a short certificate bypasses the check above, does this calculate an\nincorrect length?\n\nFor a 1-byte length encoding, p[2] would be the actual length and p[3]\nwould be the first byte of the payload, resulting in a potentially very\nlarge calculated length.\n\nIf this bogus length exceeds the buffer size, it would return an error and\ncause functional failures. If the length is within the buffer size, could\nit advance the offset incorrectly and cause parser desynchronization by\nskipping over subsequent certificates or payload data?\n\n> +\tif (plen > buflen)\n> +\t\treturn -EINVAL;\n> +\n> +\treturn plen;\n> +}\n> +EXPORT_SYMBOL_GPL(x509_get_certificate_length);\n\n[ ... ]","headers":{"Return-Path":"\n <linux-pci+bounces-54174-incoming=patchwork.ozlabs.org@vger.kernel.org>","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=QCjtCCpo;\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-54174-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=\"QCjtCCpo\"","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 4gBZc86gqtz1yJq\n\tfor <incoming@patchwork.ozlabs.org>; Fri, 08 May 2026 13:39:20 +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 049683010537\n\tfor <incoming@patchwork.ozlabs.org>; Fri,  8 May 2026 03:39:19 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 714B32F0680;\n\tFri,  8 May 2026 03:39:18 +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 4E7DD2EA498\n\tfor <linux-pci@vger.kernel.org>; Fri,  8 May 2026 03:39:18 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id B5873C2BCB0;\n\tFri,  8 May 2026 03:39:17 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778211558; cv=none;\n b=r7U0CXj4QzdCqUEVI7qV21hZFmrn1gJvnhDxL9UCh9MyNfY29lOO6CAEUYV34y2ZPk94lApkCDT6mK/pPijIkzPyWL/cp3qRNyIIecR3CbEJirEjmKF8l8T6SsKlFCmceQpO9UyXsFoN5t634aMfoLiFb+1lzjdKAlvJVrLf5h0=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778211558; c=relaxed/simple;\n\tbh=GjsIndrYHjUJOhYNTGPYDIt1nxcj+eKWeTQ63/SkBgQ=;\n\th=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date:\n\t Message-Id;\n b=af9gRr+7+ejnlqXL4i33Zwz0uoy/I6W3Vkrh4xZCrw5BAI8j5U1tPNEeHQnp7jbmy+3u8Bq2KjAidxX2j5XZlmDYUT01Ag+TH+btpZmA2jpRuOz5R2FujDjcu1wAdACwBcbJh2nQ9fRjifSEb2HgklpwEzLS6LObrLGPwgRgvJ8=","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=QCjtCCpo; 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=1778211557;\n\tbh=GjsIndrYHjUJOhYNTGPYDIt1nxcj+eKWeTQ63/SkBgQ=;\n\th=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From;\n\tb=QCjtCCpoEr3dKEen1lzLPnWuE/VpS9/yD05WfMPSGnua5KQmgD5gM9gA5sjjW/tEV\n\t L09xnrno0O+nVui8oFCV8RdM2s/ZuELEbHh6s1iQrjc3gnCtkIwl4HaVS29mQr515Y\n\t 9WXNf0QuxXB9/Og8nxZn3KGqIt2+rRAuMbFaGOCu/6lxK5uEbIsc+T8/+GXCFuAn1C\n\t xbyliIWZgPwcgZlsTIlPp2WD4NJBW8lMS0GUvEny7ex0Uqmx4/MUBVWXIk/OxRzOuW\n\t XOAhvAsqa2eTFf/UmVFnR4pWZzgl8VxJVSQlg7cJWTFG9hlWDV5KJODvX0LmZK4eaU\n\t gN4RfXr9n2wlA==","From":"sashiko-bot@kernel.org","Subject":"Re: [PATCH 04/18] X.509: Move certificate length retrieval into new\n helper","Reply-To":"sashiko@lists.linux.dev","To":"alistair23@gmail.com","Cc":"linux-pci@vger.kernel.org, ojeda@kernel.org","In-Reply-To":"<20260508031710.514574-5-alistair.francis@wdc.com>","References":"<20260508031710.514574-5-alistair.francis@wdc.com>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Date":"Fri, 08 May 2026 03:39:17 +0000","Message-Id":"<20260508033917.B5873C2BCB0@smtp.kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"<linux-pci.vger.kernel.org>","List-Subscribe":"<mailto:linux-pci+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:linux-pci+unsubscribe@vger.kernel.org>"}}]