linux-kernel - Re: [PATCH RESEND] vdpa: solidrun: Fix UB bug with devres

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <xyv4vzeq7tqq6qqfafaqxfohgmwu5tx4sb4pgg6dilpgqou32m@l4c5il7euzpb>
Date: Fri, 18 Oct 2024 16:49:33 +0200
From: Stefano Garzarella <sgarzare@...hat.com>
To: Philipp Stanner <pstanner@...hat.com>
Cc: Alvaro Karsz <alvaro.karsz@...id-run.com>, 
	"Michael S. Tsirkin" <mst@...hat.com>, Jason Wang <jasowang@...hat.com>, 
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Eugenio Pérez <eperezma@...hat.com>, 
	virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org, stable@...r.kernel.org, 
	Christophe JAILLET <christophe.jaillet@...adoo.fr>, Andy Shevchenko <andy@...nel.org>
Subject: Re: [PATCH RESEND] vdpa: solidrun: Fix UB bug with devres

On Wed, Oct 16, 2024 at 09:25:54AM +0200, Philipp Stanner wrote:
>In psnet_open_pf_bar() and snet_open_vf_bar() a string later passed to
>pcim_iomap_regions() is placed on the stack. Neither
>pcim_iomap_regions() nor the functions it calls copy that string.
>
>Should the string later ever be used, this, consequently, causes
>undefined behavior since the stack frame will by then have disappeared.
>
>Fix the bug by allocating the strings on the heap through
>devm_kasprintf().
>
>Cc: stable@...r.kernel.org	# v6.3
>Fixes: 51a8f9d7f587 ("virtio: vdpa: new SolidNET DPU driver.")
>Reported-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
>Closes: https://lore.kernel.org/all/74e9109a-ac59-49e2-9b1d-d825c9c9f891@wanadoo.fr/
>Suggested-by: Andy Shevchenko <andy@...nel.org>
>Signed-off-by: Philipp Stanner <pstanner@...hat.com>
>---
> drivers/vdpa/solidrun/snet_main.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)

LGTM!

Reviewed-by: Stefano Garzarella <sgarzare@...hat.com>

>
>diff --git a/drivers/vdpa/solidrun/snet_main.c b/drivers/vdpa/solidrun/snet_main.c
>index 99428a04068d..c8b74980dbd1 100644
>--- a/drivers/vdpa/solidrun/snet_main.c
>+++ b/drivers/vdpa/solidrun/snet_main.c
>@@ -555,7 +555,7 @@ static const struct vdpa_config_ops snet_config_ops = {
>
> static int psnet_open_pf_bar(struct pci_dev *pdev, struct psnet *psnet)
> {
>-	char name[50];
>+	char *name;
> 	int ret, i, mask = 0;
> 	/* We don't know which BAR will be used to communicate..
> 	 * We will map every bar with len > 0.
>@@ -573,7 +573,10 @@ static int psnet_open_pf_bar(struct pci_dev *pdev, struct psnet *psnet)
> 		return -ENODEV;
> 	}
>
>-	snprintf(name, sizeof(name), "psnet[%s]-bars", pci_name(pdev));
>+	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "psnet[%s]-bars", pci_name(pdev));
>+	if (!name)
>+		return -ENOMEM;
>+
> 	ret = pcim_iomap_regions(pdev, mask, name);
> 	if (ret) {
> 		SNET_ERR(pdev, "Failed to request and map PCI BARs\n");
>@@ -590,10 +593,13 @@ static int psnet_open_pf_bar(struct pci_dev *pdev, struct psnet *psnet)
>
> static int snet_open_vf_bar(struct pci_dev *pdev, struct snet *snet)
> {
>-	char name[50];
>+	char *name;
> 	int ret;
>
>-	snprintf(name, sizeof(name), "snet[%s]-bar", pci_name(pdev));
>+	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "snet[%s]-bars", pci_name(pdev));
>+	if (!name)
>+		return -ENOMEM;
>+
> 	/* Request and map BAR */
> 	ret = pcim_iomap_regions(pdev, BIT(snet->psnet->cfg.vf_bar), name);
> 	if (ret) {
>-- 
>2.46.1
>
>