e100: Correct firmware memory leak

Submitted by Simon Kagstrom on May 23, 2011, 7:07 a.m.

Details

Message ID 20110523090700.68cdf15a@marrow.netinsight.se
State Awaiting Upstream
Delegated to: David Miller
Headers show

Commit Message

Simon Kagstrom May 23, 2011, 7:07 a.m.
kmemcheck reports

unreferenced object 0xcfaf4f00 (size 32):
  comm "ifconfig", pid 682, jiffies 87369
  backtrace:
    [<c00252b4>] save_stack_trace+0x20/0x24
    [<c00a5f98>] create_object+0x118/0x20c
    [<c00a61a8>] kmemleak_alloc+0x40/0x84
    [<c00a2de4>] kmem_cache_alloc+0x114/0x1a4
    [<c016ce50>] _request_firmware+0x3c/0x540
    [<c016d3f8>] request_firmware+0x14/0x18
    [<c0170774>] e100_hw_init+0xf0/0x3d8
    [<c0171340>] e100_up+0x38/0x16c
    [<c0171494>] e100_open+0x20/0x54
    [<c019779c>] dev_open+0xcc/0x134
    [<c0196cf0>] dev_change_flags+0xb0/0x190
    [<c01e0998>] devinet_ioctl+0x2f0/0x6fc
    [<c01e1dc4>] inet_ioctl+0xcc/0x104
    [<c01861d8>] sock_ioctl+0x200/0x25c
    [<c00b4cbc>] vfs_ioctl+0x34/0x78
    [<c00b5400>] do_vfs_ioctl+0x4e4/0x53c

when the interface is taken up because the firmware is loaded with
request_firmware, but never released in the callback where it's used,
so fix that.

Problem introduced in

  9ac32e1bc0518b01b47dd34a733dce8634a38ed3

as far as I can tell.

Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
 drivers/net/e100.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Comments

Simon Kagstrom May 30, 2011, 9:05 a.m.
Hi again,

On Mon, 23 May 2011 09:07:00 +0200
Simon Kagstrom <simon.kagstrom@netinsight.net> wrote:

> kmemcheck reports
> 
> unreferenced object 0xcfaf4f00 (size 32):
>   comm "ifconfig", pid 682, jiffies 87369
>   backtrace:
>     [<c00252b4>] save_stack_trace+0x20/0x24
>     [<c00a5f98>] create_object+0x118/0x20c
>     [<c00a61a8>] kmemleak_alloc+0x40/0x84
>     [<c00a2de4>] kmem_cache_alloc+0x114/0x1a4
>     [<c016ce50>] _request_firmware+0x3c/0x540
>     [<c016d3f8>] request_firmware+0x14/0x18
>     [<c0170774>] e100_hw_init+0xf0/0x3d8
>     [<c0171340>] e100_up+0x38/0x16c
>     [<c0171494>] e100_open+0x20/0x54
>     [<c019779c>] dev_open+0xcc/0x134
>     [<c0196cf0>] dev_change_flags+0xb0/0x190
>     [<c01e0998>] devinet_ioctl+0x2f0/0x6fc
>     [<c01e1dc4>] inet_ioctl+0xcc/0x104
>     [<c01861d8>] sock_ioctl+0x200/0x25c
>     [<c00b4cbc>] vfs_ioctl+0x34/0x78
>     [<c00b5400>] do_vfs_ioctl+0x4e4/0x53c
> [..]
> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
> index b0aa9e6..f2b44ef 100644
> --- a/drivers/net/e100.c
> +++ b/drivers/net/e100.c
> @@ -1320,6 +1320,8 @@ static void e100_setup_ucode(struct nic *nic, struct cb *cb,
>  	cb->u.ucode[min_size] |= cpu_to_le32((BUNDLESMALL) ? 0xFFFF : 0xFF80);
>  
>  	cb->command = cpu_to_le16(cb_ucode | cb_el);
> +
> +	release_firmware(fw);
>  }

This patch can be dropped, I've made a mistake. I think there is a
memory leak when the driver is unloaded, since  nic->fw  is never
released, but that has to be solved in another way.

It's also not as serious since it only happens on module unload, not on
taking down the interface as indicated above.

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

Patch hide | download patch | download mbox

diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index b0aa9e6..f2b44ef 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -1320,6 +1320,8 @@  static void e100_setup_ucode(struct nic *nic, struct cb *cb,
 	cb->u.ucode[min_size] |= cpu_to_le32((BUNDLESMALL) ? 0xFFFF : 0xFF80);
 
 	cb->command = cpu_to_le16(cb_ucode | cb_el);
+
+	release_firmware(fw);
 }
 
 static inline int e100_load_ucode_wait(struct nic *nic)