diff mbox

[dpdk-dev,3/3] pci: fix crash caused by invaild memory write

Message ID 1487599487-12574-3-git-send-email-hanxueluo@126.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers show

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

hanxueluo@126.com Feb. 20, 2017, 2:04 p.m. UTC 
From: Huanle Han <hanxueluo@gmail.com>

rte_eal_dev_detach() didn't remove dev from dev_device_list
after free the dev. So the following attached dev wrote to
the freed memory (tailq entry of previous dev) in below stack:

== Invalid write of size 8
==    at 0x43A9CE: rte_eal_device_insert (eal_common_dev.c:71)
==    by 0x42ED9E: pci_scan_one (eal_pci.c:365)
==    by 0x42EF4D: pci_update_device (eal_pci.c:391)
==    by 0x437F59: rte_eal_pci_probe_one (eal_common_pci.c:357)
==    by 0x43AB16: rte_eal_dev_attach (eal_common_dev.c:117)
==    by 0x45B3AA: rte_eth_dev_attach (rte_ethdev.c:489)
==    ...

Signed-off-by: Huanle Han <hanxueluo@gmail.com>
---
 lib/librte_eal/common/eal_common_pci.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Yuanhan Liu Feb. 22, 2017, 2:36 a.m. UTC | #1 
cc the EAL maintainer, David Marchand.

On Mon, Feb 20, 2017 at 10:04:47PM +0800, hanxueluo@126.com wrote:
> From: Huanle Han <hanxueluo@gmail.com>
> 
> rte_eal_dev_detach() didn't remove dev from dev_device_list
> after free the dev. So the following attached dev wrote to
> the freed memory (tailq entry of previous dev) in below stack:
> 
> == Invalid write of size 8
> ==    at 0x43A9CE: rte_eal_device_insert (eal_common_dev.c:71)
> ==    by 0x42ED9E: pci_scan_one (eal_pci.c:365)
> ==    by 0x42EF4D: pci_update_device (eal_pci.c:391)
> ==    by 0x437F59: rte_eal_pci_probe_one (eal_common_pci.c:357)
> ==    by 0x43AB16: rte_eal_dev_attach (eal_common_dev.c:117)
> ==    by 0x45B3AA: rte_eth_dev_attach (rte_ethdev.c:489)
> ==    ...
> 
> Signed-off-by: Huanle Han <hanxueluo@gmail.com>

The two minor nits also apply here, besides that,

Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

	--yliu
Thomas Monjalon May 1, 2017, 10:59 p.m. UTC | #2 
22/02/2017 03:36, Yuanhan Liu:
> On Mon, Feb 20, 2017 at 10:04:47PM +0800, hanxueluo@126.com wrote:
> > From: Huanle Han <hanxueluo@gmail.com>
> > 
> > rte_eal_dev_detach() didn't remove dev from dev_device_list
> > after free the dev. So the following attached dev wrote to
> > the freed memory (tailq entry of previous dev) in below stack:
> > 
> > == Invalid write of size 8
> > ==    at 0x43A9CE: rte_eal_device_insert (eal_common_dev.c:71)
> > ==    by 0x42ED9E: pci_scan_one (eal_pci.c:365)
> > ==    by 0x42EF4D: pci_update_device (eal_pci.c:391)
> > ==    by 0x437F59: rte_eal_pci_probe_one (eal_common_pci.c:357)
> > ==    by 0x43AB16: rte_eal_dev_attach (eal_common_dev.c:117)
> > ==    by 0x45B3AA: rte_eth_dev_attach (rte_ethdev.c:489)
> > ==    ...
> > 
> > Signed-off-by: Huanle Han <hanxueluo@gmail.com>
> 
> The two minor nits also apply here, besides that,
> 
> Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Nack

The right fix is to remove the functions insert and remove.
The list dev_device_list is not used anymore.

I will make a patch.
diff mbox

Patch

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 72547bd..022fdc7 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -393,6 +393,7 @@  rte_eal_pci_detach(const struct rte_pci_addr *addr)
 			goto err_return;
 
 		TAILQ_REMOVE(&pci_device_list, dev, next);
+		rte_eal_device_remove(&dev->device);
 		free(dev);
 		return 0;
 	}