Skip to contentSkip to conversation

8139too in linux-3.18.0: some potential bugs

PATCH In review Jia-Ju Bai 3 participants 4 messages Dec 20, 2014 – Dec 21, 2014

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Jia-Ju Bai

author
#0 lore raw

Sorry, I know your meaning.

Signed-off-by Jia-Ju Bai <[email protected]>
(quoted diff) +0 -0

b/drivers/net/ethernet/realtek/8139too.c
index 007b38c..f277c5f 100644

drivers/net/ethernet/realtek/8139too.c +0 -0
@@ -782,11 +782,11 @@ static struct net_device *rtl8139_init_board(struct

pci_dev *pdev)
rc = pci_enable_device (pdev);
if (rc)
goto err_out;
+ disable_dev_on_err = 1;

rc = pci_request_regions (pdev, DRV_NAME); if (rc)
goto err_out;
- disable_dev_on_err = 1;

pci_set_master (pdev);

@@ -1098,6 +1098,7 @@ static int rtl8139_init_one(struct pci_dev *pdev,
return 0;

err_out:
+ netif_napi_del(&tp->napi);
__rtl8139_cleanup_dev (dev);
pci_disable_device (pdev);
return i;
@@ -1112,6 +1113,7 @@ static void rtl8139_remove_one(struct pci_dev *pdev)
assert (dev != NULL);

cancel_delayed_work_sync(&tp->thread);
+ netif_napi_del(&tp->napi);

unregister_netdev (dev);

-----邮件原件-----
发件人: Sergei Shtylyov [mailto:[email protected]] 发送时间: 2014年12月20日 21:48
收件人: Jia-Ju Bai; [email protected]; [email protected]; [email protected]
主题: Re: [PATCH] 8139too in linux-3.18.0: some potential bugs

Quoted 10 lines · Jia-Ju Bai

I have actually tested 8139too driver on the real hardware(Realtek RTL8139D PCI Ethernet Controller), and find some potential bugs:
The target file is drivers/net/ethernet/realtek/8139too.c, which is used to build 8139too.ko.

(1) In the normal process of 8139too, netif_napi_add is called in rtl8139_init_one, but netif_napi_del is not called in rtl8139_remove_one. However, many other ethernet card drivers call them in pairs, even in the error handling paths, such as r8169 and igb.

Quoted 6 lines

(2) In the normal process of 8139too, pci_enable_device and pci_disable_device are called in pairs in rtl8139_init_board(in rtl8139_init_one) and rtl8139_remove_one. However, when pci_enable_device has been called and pci_request_regions is failed in rtl8139_init_board, "err_out" segment in rtl8139_init_board is executed immediately to exit, but pci_disable_device is not called because

"disable_dev_on_err = 0".

Again, please fix one issue per patch.

Quoted 2 lines

Meanwhile, I also write the patch to fix the bugs. I have run the patch on the hardware, it can work normally and fix the above bugs.

Too bad you didn't provide a sign-off...

WBR, Sergei

#1 lore raw
Quoted 1 line · Jia-Ju Bai

Sorry, I know your meaning.

No, apparently you don't. Please:

1) don't send patches with the description stripped;
2) split your patch into 2 patches to solve one issue per patch.

Quoted 1 line
Signed-off-by Jia-Ju Bai <[email protected]>

WBR, Sergei

author
#2 lore raw

PATCH v2 1/2 8139too in linux-3.18.0: Fix the lack of pci_disable_device

When pci_request_regions is failed in rtl8139_init_board, pci_disable_device is not called to disable the device which are enabled by pci_enable_device, because disable_dev_on_err is not assigned 1. This patch fix this problem.

Signed-off-by Jia-Ju Bai <[email protected]>

---
drivers/net/ethernet/realtek/8139too.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/realtek/8139too.c +0 -0
index 007b38c..49bbcf3 100644
@@ -783,10 +783,10 @@ static struct net_device *rtl8139_init_board(struct pci_dev *pdev)

if (rc)
goto err_out;

+ disable_dev_on_err = 1;
rc = pci_request_regions (pdev, DRV_NAME);
if (rc)
goto err_out;
- disable_dev_on_err = 1;

pci_set_master (pdev);

maintainer
#3 lore raw

Re: [PATCH v2 1/2] 8139too in linux-3.18.0: Fix the lack of pci_disable_device

From: Jia-Ju Bai <[email protected]>
Date: Sun, 21 Dec 2014 15:36:43 +0800

Quoted 7 lines

When pci_request_regions is failed in rtl8139_init_board, pci_disable_device is not called to disable the device which are enabled by pci_enable_device, because disable_dev_on_err is not assigned 1.
This patch fix this problem.

Signed-off-by Jia-Ju Bai <[email protected]>

Your patches are severely corrupted by your email client, and thus are not usable.