Re: [PATCH v3] 3c59x: Fix memory leaks in vortex_open

[Jia-Ju Bai <baijiaju1990@163.com>]

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.