netdev - Re: [PATCH v3] 3c59x: Fix memory leaks in vortex

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Wed, 24 Dec 2014 10:12:58 +0800
From:	Jia-Ju Bai <baijiaju1990@....com>
To:	Neil Horman <nhorman@...driver.com>
CC:	davem@...emloft.net, ebiederm@...ssion.com,
	dingtianhong@...wei.com, paul.gortmaker@...driver.com,
	justinvanwijngaarden@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH v3] 3c59x: Fix memory leaks in vortex_open

On 12/23/2014 11:43 PM, Neil Horman wrote:
> No, I don't think so.  vortex_close predicates each free with a NULL check, so
> if its not been allocated, it shouldn't be freed.  vortex_close also puts the
> adapter back into a known state (undoing all the setup that vortex_open does).
> I really think its better to go with the proper close path than just unwinding
> the allocation
>
> Neil
>

Firstly, I run my match on the real hardware(3com 3c905B 100Base
PCI Ethernet Controller) and make vortex_up failed on purpose
(make "pci_enable_device" in vortex_up failed). During runtime, the driver
works well and memory leaks are fixed.

Secondly, I revise the code according to your opinion:

         retval = vortex_up(dev);
         if (!retval)
             goto out;

+      vortex_close(dev);
+      return -ENOMEM;

Then I repeat my experiment, but system hang occurs!

After adding some "printk"s into the code and running the driver, I find
the problem's source:
vortex_close calls vortex_down in runtime, and vortex_down calls
"del_timer_sync(&vp->rx_oom_timer);" in the code. However, I make
"pci_enable_device" failed in vortext_up to let vortex_up return an
error code directly, but "vp->rx_oom_timer" is initialized only by
"init_timer" after "pci_enable_device". Thus when
"del_timer_sync(&vp->rx_oom_timer);" is called in vortex_down,
a null dereference may occur.
Moreover, only "pci_enable_device" can make vortex_up failed.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html