On 05/15/2018 06:31 PM, Jerin Jacob wrote:
-----Original Message-----
Date: Tue, 15 May 2018 18:19:19 +0800
From: Andy Green <a...@warmcat.com>
To: Jerin Jacob <jerin.ja...@caviumnetworks.com>, dev@dpdk.org
CC: tho...@monjalon.net, ferruh.yi...@intel.com, Pablo de Lara
<pablo.de.lara.gua...@intel.com>
Subject: Re: [dpdk-dev] [PATCH] bus/pci: fix driver name string manipulation
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
Thunderbird/52.7.0
On 05/15/2018 06:03 PM, Jerin Jacob wrote:
sizeof(dri_name) is 8B on 64Bit systems.The intended operation is coping
the string after '/' from the string `name`.
This bug is not letting to probe any device string >8B hence results in
the testpmd error("No ethernet devices found) on some PMDs.
You are right... but...
Cc: Andy Green <a...@warmcat.com>
Cc: Pablo de Lara <pablo.de.lara.gua...@intel.com>
Fixes: fe5f777b538 ("bus/pci: replace strncpy by strlcpy")
Signed-off-by: Jerin Jacob <jerin.ja...@caviumnetworks.com>
---
drivers/bus/pci/linux/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index a73ee49c2..cd45875b1 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -54,7 +54,7 @@ pci_get_kernel_driver_by_path(const char *filename, char
*dri_name)
name = strrchr(path, '/');
if (name) {
- strlcpy(dri_name, name + 1, sizeof(dri_name));
+ strlcpy(dri_name, name + 1, strlen(name));
... this fix is no good. The underlying problem is the length of dri_name
is not getting passed into this function... it just doesn't know how much of
dri_name is safe to use. Telling it to use the strlen() of something
unrelated is going to make buffer overflows possible.
In this case, already there is a check for following in the code. So it is fine
either way.
path[count] = '\0';
name = strrchr(path, '/');
I understand. But if any of that code changes, the copy can overrun.
It's more robust if the copy limit is directly referenced to the
destination size.
I sent a patch to the list a few hours ago that amends this function to take
the allocated length of dri_name, and sets the limit for the strlcpy() to
that, so no matter what turns up in name it's not possible to blow past
dri_name allocation.
[dpdk-dev] [PATCH] bus/pci: correct the earlier strlcpy conversion​
I am fine with taking any of the of the patch. Please improve the patch subject
and bug description in case if you prefer to take your original patch.
OK. Or the maintainer is welcome to change the commit title how he
likes directly.
-Andy
-Andy
return 0;
}
