Patchwork e100: Correct firmware memory leak

login
register
mail settings
Submitter Simon Kagstrom
Date May 23, 2011, 7:07 a.m.
Message ID <20110523090700.68cdf15a@marrow.netinsight.se>
Download mbox | patch
Permalink /patch/96832/
State Awaiting Upstream
Delegated to: David Miller
Headers show

Comments

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(-)
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

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)