[{"id":3688142,"web_url":"http://patchwork.ozlabs.org/comment/3688142/","msgid":"<20260508035806.0EE8DC2BCB0@smtp.kernel.org>","list_archive_url":null,"date":"2026-05-08T03:58:05","subject":"Re: [PATCH 10/18] PCI/CMA: Validate Subject Alternative Name in\n certificates","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 6 potential issue(s) to consider:\n- [High] Valid devices with a DSN of 0 are incorrectly rejected\n- [High] SSVID and SSID fields are incorrectly omitted for PCIe bridges\n- [High] Torn read of device identity prevents reauthentication after runtime mutations\n- [High] Kbuild linkage failure when compiled as a loadable module\n- [Medium] Chaining `snprintf` without length checks risks logic errors and out-of-bounds stack reads\n- [Medium] Strict case-sensitive comparison of the Subject Alternative Name risks rejecting valid certificates\n--\n\ncommit 72435325efe665bb825f47731ca5c0081b7acc14\nAuthor: Lukas Wunner \nSubject: PCI/CMA: Validate Subject Alternative Name in certificates\n\nThis commit introduces functionality to validate the Subject Alternative Name\n(SAN) in PCIe Leaf Certificates. It parses the SAN using an ASN.1 decoder\nand verifies that the identity encoded in the certificate matches the\ndevice's identity in Config Space.\n\n> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile\n> index 16abfd0e17e19..15512512fce7d 100644\n> --- a/drivers/pci/Makefile\n> +++ b/drivers/pci/Makefile\n[ ... ]\n> -obj-$(CONFIG_PCI_CMA)\t\t+= cma.o\n> +obj-$(CONFIG_PCI_CMA)\t\t+= cma.o cma.asn1.o\n> +$(obj)/cma.o:\t\t\t$(obj)/cma.asn1.h\n> +$(obj)/cma.asn1.o:\t\t$(obj)/cma.asn1.c $(obj)/cma.asn1.h\n\nIf CONFIG_PCI_CMA is configured as a loadable module, will Kbuild compile\ntwo separate modules (cma.ko and cma.asn1.ko)?\n\nBecause the ASN.1 compiler does not export the cma_decoder symbol, would\nthe cma.ko module fail to link during modpost with an undefined symbol\nerror?\n\nShould these objects be bundled into a composite object instead? For\nexample:\n\ncma_module-y := cma.o cma.asn1.o\nobj-$(CONFIG_PCI_CMA) += cma_module.o\n\n> diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c\n> index 998fde6366fb8..ee186f3619409 100644\n> --- a/drivers/pci/cma.c\n> +++ b/drivers/pci/cma.c\n[ ... ]\n> +int pci_cma_note_san(void *context, size_t hdrlen, unsigned char tag,\n> +\t\t const void *value, size_t vlen)\n> +{\n> +\tstruct pci_cma_x509_context *ctx = context;\n> +\n> +\t/* These aren't the drOIDs we're looking for. */\n> +\tif (ctx->last_oid != OID_CMA)\n> +\t\treturn 0;\n> +\n> +\tif (tag != ASN1_UTF8STR ||\n> +\t vlen != ctx->expected_len ||\n> +\t memcmp(value, ctx->expected_name, vlen) != 0) {\n\nDoes this perform a strict case-sensitive comparison of the Subject\nAlternative Name?\n\nIf a device vendor provisions the certificate using uppercase hexadecimal\nletters (a common standard for hardware IDs), could this strict memcmp()\nerroneously reject a valid device?\n\n> +\t\tpci_err(ctx->pdev, \"Leaf certificate of slot %u \"\n> +\t\t\t\"has invalid Subject Alternative Name\\n\", ctx->slot);\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tctx->found = true;\n> +\n> +\treturn 0;\n> +}\n> +\n> +static unsigned int pci_cma_construct_san(struct pci_dev *pdev, char *name)\n> +{\n> +\tunsigned int len;\n> +\tu64 serial;\n> +\n> +\tlen = snprintf(name, CMA_NAME_MAX,\n> +\t\t \"Vendor=%04hx:Device=%04hx:CC=%06x:REV=%02hhx\",\n> +\t\t pdev->vendor, pdev->device, pdev->class, pdev->revision);\n\nThe commit message mentions handling devices that mutate their identity\nat runtime, but this string is constructed using cached software state\nfrom probe time (like pdev->vendor).\n\nSince pci_get_dsn() reads the live hardware state, could a mutated device\nresult in an inconsistent string containing a stale vendor/device ID but\na new DSN? Will reauthentication fail without a full PCI rescan?\n\nAdditionally, snprintf() returns the number of characters that would have\nbeen written, not the actual bytes written.\n\nIf the inputs ever exceed the 79-byte CMA_NAME_MAX buffer (for instance,\nif a quirk mutates pdev->class to exceed 6 hex characters), the string\nwill be truncated but len will reflect the untruncated size.\n\nCould this cause the later memcmp() check in pci_cma_note_san() to read\nout of bounds of the ctx->expected_name stack buffer? Should scnprintf()\nbe used here instead to track the actual length written?\n\n> +\n> +\tif (pdev->hdr_type == PCI_HEADER_TYPE_NORMAL)\n> +\t\tlen += snprintf(name + len, CMA_NAME_MAX - len,\n> +\t\t\t\t\":SSVID=%04hx:SSID=%04hx\",\n> +\t\t\t\tpdev->subsystem_vendor, pdev->subsystem_device);\n\nPCIe Type 1 devices (bridges) can also implement the Subsystem Vendor ID\ncapability.\n\nIf a Type 1 bridge implements this capability and includes these fields in\nits certificate as required by the specification, will this condition\nunconditionally omit them and cause validation to fail?\n\n> +\n> +\tserial = pci_get_dsn(pdev);\n> +\tif (serial)\n> +\t\tlen += snprintf(name + len, CMA_NAME_MAX - len,\n> +\t\t\t\t\":%016llx\", serial);\n> +\n> +\treturn len;\n> +}\n\nLooking at pci_get_dsn(), it returns 0 both when the DSN capability is\nmissing and when the device's legitimate serial number is 0.\n\nIf a device implements the capability with a valid DSN of 0, will this\ncheck evaluate to false, omit the field, and reject a valid device?\n\nWould it be safer to explicitly check for the capability's presence via\npci_find_ext_capability() instead?\n\nAlso, pci_get_dsn() performs two separate 32-bit reads without a lock:\n\npci_read_config_dword(dev, pos, &dword);\ndsn = (u64)dword;\npci_read_config_dword(dev, pos + 4, &dword);\ndsn |= ((u64)dword) << 32;\n\nCould a concurrent mutation of the device identity result in a torn\n64-bit read during this check?","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=FeFmUgv/;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=linux-pci+bounces-54184-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=\"FeFmUgv/\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::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 4gBb2y3C2Wz1yCg\n\tfor ; Fri, 08 May 2026 13:59:06 +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 B7CD4301412F\n\tfor ; Fri, 8 May 2026 03:58:07 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id F230C2C3266;\n\tFri, 8 May 2026 03:58:06 +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 CF9CD28725B\n\tfor ; Fri, 8 May 2026 03:58:06 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 0EE8DC2BCB0;\n\tFri, 8 May 2026 03:58:06 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778212686; cv=none;\n b=andJB7jOJbQQ9fzMXhgeUYZAwVEKBe/UPX+vuviRu+Kv+T4GsSfdB6gaAkUOtKAMJ1npY7Q53trVouQmshqxI3FFJDFXsi6S2zYdSqePGdiXx1pn/REH8iqrZI5cmjShMHEdHsdmbLzYKvBuRjkTQOQgdzqjYHvFzbjyqhqlCgo=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778212686; c=relaxed/simple;\n\tbh=Rb6o2pAgryP/W1ucJ1j0Y1K+zUjA9Nncf0PWkb75PPo=;\n\th=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date:\n\t Message-Id;\n b=bk9jrdyLSzzlh2qNrsxy+Ma2eToLUYCp8/fYpIRAYFgM6qyVmFBaZ7BdUgKpoLQ95iDGmnSIIbwH82Km1/oUcTFTbz8RjIfb1+SPiGPaEYgADt4aTMJRUtUgqQIPO1JqOvwEkoP8N2Q0H0BkkZwjOo6zAhddXeYw/JVwPzlb5W4=","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=FeFmUgv/; 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=1778212686;\n\tbh=Rb6o2pAgryP/W1ucJ1j0Y1K+zUjA9Nncf0PWkb75PPo=;\n\th=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From;\n\tb=FeFmUgv/5s7QSzA9d3jTuphN3bQgxmbTzAc0XrCeXcH2emQzccvCGQWYELHD8KvlE\n\t Bq2lpfkxEG1mWM+1vtyJUliBUEnLU9T/LuGuMysbHVl+UhcGiCmSqVaY9UoDV7s9u8\n\t bSNKkmK9vWmQxcKftbjuJmP2qdk5U0ylnCdcInMpissQB2CsbDoYve/LmRkHJklLzd\n\t YOAuK3bVXQg7eOXLd64wR/SF0YLBu/ejXlMburXYOJWzP3qLfzoAjy+I0Sub4jmVB9\n\t YLpf1Hv6ZbdJ/blK151x0pSDWwCteouh5RM9h014P0r9Cr14/mWOqnnOKgbkNdKv0W\n\t Sw2BXyTzqgDvg==","From":"sashiko-bot@kernel.org","Subject":"Re: [PATCH 10/18] PCI/CMA: Validate Subject Alternative Name in\n certificates","Reply-To":"sashiko@lists.linux.dev","To":"alistair23@gmail.com","Cc":"linux-pci@vger.kernel.org, ojeda@kernel.org","In-Reply-To":"<20260508031710.514574-11-alistair.francis@wdc.com>","References":"<20260508031710.514574-11-alistair.francis@wdc.com>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Date":"Fri, 08 May 2026 03:58:05 +0000","Message-Id":"<20260508035806.0EE8DC2BCB0@smtp.kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":""}}]