| Message ID | 20220511103604.37962-2-ihuguet@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | sfc: fix mtd memleak and simplify list handling | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 22 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
The same patch was submitted earlier, see https://lore.kernel.org/netdev/20220510153619.32464-1-ap420073@gmail.com/ Martin On Wed, May 11, 2022 at 12:36:03PM +0200, Íñigo Huguet wrote: > In some cases there is no mtd partitions that can be probed, so the mtd > partitions list stays empty. This happens, for example, in SFC9220 > devices on the second port of the NIC. > > The memory for the mtd partitions is deallocated in efx_mtd_remove, > recovering the address of the first element of efx->mtd_list and then > deallocating it. But if the list is empty, the address passed to kfree > doesn't point to the memory allocated for the mtd partitions, but to the > list head itself. Despite this hasn't caused other problems other than > the memory leak, this is obviously incorrect. > > This patch deallocates the memory during mtd_probe in the case that > there are no probed partitions, avoiding the leak. > > This was detected with kmemleak, output example: > unreferenced object 0xffff88819cfa0000 (size 46560): > comm "kworker/0:2", pid 48435, jiffies 4364987018 (age 45.924s) > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > backtrace: > [<000000000f8e92d9>] kmalloc_order_trace+0x19/0x130 > [<0000000042a03844>] efx_ef10_mtd_probe+0x12d/0x320 [sfc] > [<000000004555654f>] efx_pci_probe.cold+0x4e1/0x6db [sfc] > [<00000000b03d5126>] local_pci_probe+0xde/0x170 > [<00000000376cc8d9>] work_for_cpu_fn+0x51/0xa0 > [<00000000141f8de9>] process_one_work+0x8cb/0x1590 > [<00000000cb2d8065>] worker_thread+0x707/0x1010 > [<000000001ef4b9f6>] kthread+0x364/0x420 > [<0000000014767137>] ret_from_fork+0x22/0x30 > > Fixes: 8127d661e77f ("sfc: Add support for Solarflare SFC9100 family") > Reported-by: Liang Li <liali@redhat.com> > Signed-off-by: Íñigo Huguet <ihuguet@redhat.com> > --- > drivers/net/ethernet/sfc/ef10.c | 5 +++++ > drivers/net/ethernet/sfc/siena/siena.c | 5 +++++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c > index c9ee5011803f..15a229731296 100644 > --- a/drivers/net/ethernet/sfc/ef10.c > +++ b/drivers/net/ethernet/sfc/ef10.c > @@ -3579,6 +3579,11 @@ static int efx_ef10_mtd_probe(struct efx_nic *efx) > n_parts++; > } > > + if (n_parts == 0) { > + kfree(parts); > + return 0; > + } > + > rc = efx_mtd_add(efx, &parts[0].common, n_parts, sizeof(*parts)); > fail: > if (rc) > diff --git a/drivers/net/ethernet/sfc/siena/siena.c b/drivers/net/ethernet/sfc/siena/siena.c > index 741313aff1d1..32467782e8ef 100644 > --- a/drivers/net/ethernet/sfc/siena/siena.c > +++ b/drivers/net/ethernet/sfc/siena/siena.c > @@ -943,6 +943,11 @@ static int siena_mtd_probe(struct efx_nic *efx) > nvram_types >>= 1; > } > > + if (n_parts == 0) { > + kfree(parts); > + return 0; > + } > + > rc = siena_mtd_get_fw_subtypes(efx, parts, n_parts); > if (rc) > goto fail; > -- > 2.34.1
On Wed, May 11, 2022 at 2:58 PM Martin Habets <habetsm.xilinx@gmail.com> wrote: > > The same patch was submitted earlier, see > https://lore.kernel.org/netdev/20220510153619.32464-1-ap420073@gmail.com/ Seriously? It has been there for 9 years and 2 persons find it and send the fix with hours of difference? Is someone spying me? Please review the other one and if it's OK, I will send it alone. > > Martin > > On Wed, May 11, 2022 at 12:36:03PM +0200, Íñigo Huguet wrote: > > In some cases there is no mtd partitions that can be probed, so the mtd > > partitions list stays empty. This happens, for example, in SFC9220 > > devices on the second port of the NIC. > > > > The memory for the mtd partitions is deallocated in efx_mtd_remove, > > recovering the address of the first element of efx->mtd_list and then > > deallocating it. But if the list is empty, the address passed to kfree > > doesn't point to the memory allocated for the mtd partitions, but to the > > list head itself. Despite this hasn't caused other problems other than > > the memory leak, this is obviously incorrect. > > > > This patch deallocates the memory during mtd_probe in the case that > > there are no probed partitions, avoiding the leak. > > > > This was detected with kmemleak, output example: > > unreferenced object 0xffff88819cfa0000 (size 46560): > > comm "kworker/0:2", pid 48435, jiffies 4364987018 (age 45.924s) > > hex dump (first 32 bytes): > > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > > backtrace: > > [<000000000f8e92d9>] kmalloc_order_trace+0x19/0x130 > > [<0000000042a03844>] efx_ef10_mtd_probe+0x12d/0x320 [sfc] > > [<000000004555654f>] efx_pci_probe.cold+0x4e1/0x6db [sfc] > > [<00000000b03d5126>] local_pci_probe+0xde/0x170 > > [<00000000376cc8d9>] work_for_cpu_fn+0x51/0xa0 > > [<00000000141f8de9>] process_one_work+0x8cb/0x1590 > > [<00000000cb2d8065>] worker_thread+0x707/0x1010 > > [<000000001ef4b9f6>] kthread+0x364/0x420 > > [<0000000014767137>] ret_from_fork+0x22/0x30 > > > > Fixes: 8127d661e77f ("sfc: Add support for Solarflare SFC9100 family") > > Reported-by: Liang Li <liali@redhat.com> > > Signed-off-by: Íñigo Huguet <ihuguet@redhat.com> > > --- > > drivers/net/ethernet/sfc/ef10.c | 5 +++++ > > drivers/net/ethernet/sfc/siena/siena.c | 5 +++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c > > index c9ee5011803f..15a229731296 100644 > > --- a/drivers/net/ethernet/sfc/ef10.c > > +++ b/drivers/net/ethernet/sfc/ef10.c > > @@ -3579,6 +3579,11 @@ static int efx_ef10_mtd_probe(struct efx_nic *efx) > > n_parts++; > > } > > > > + if (n_parts == 0) { > > + kfree(parts); > > + return 0; > > + } > > + > > rc = efx_mtd_add(efx, &parts[0].common, n_parts, sizeof(*parts)); > > fail: > > if (rc) > > diff --git a/drivers/net/ethernet/sfc/siena/siena.c b/drivers/net/ethernet/sfc/siena/siena.c > > index 741313aff1d1..32467782e8ef 100644 > > --- a/drivers/net/ethernet/sfc/siena/siena.c > > +++ b/drivers/net/ethernet/sfc/siena/siena.c > > @@ -943,6 +943,11 @@ static int siena_mtd_probe(struct efx_nic *efx) > > nvram_types >>= 1; > > } > > > > + if (n_parts == 0) { > > + kfree(parts); > > + return 0; > > + } > > + > > rc = siena_mtd_get_fw_subtypes(efx, parts, n_parts); > > if (rc) > > goto fail; > > -- > > 2.34.1 >
diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c index c9ee5011803f..15a229731296 100644 --- a/drivers/net/ethernet/sfc/ef10.c +++ b/drivers/net/ethernet/sfc/ef10.c @@ -3579,6 +3579,11 @@ static int efx_ef10_mtd_probe(struct efx_nic *efx) n_parts++; } + if (n_parts == 0) { + kfree(parts); + return 0; + } + rc = efx_mtd_add(efx, &parts[0].common, n_parts, sizeof(*parts)); fail: if (rc) diff --git a/drivers/net/ethernet/sfc/siena/siena.c b/drivers/net/ethernet/sfc/siena/siena.c index 741313aff1d1..32467782e8ef 100644 --- a/drivers/net/ethernet/sfc/siena/siena.c +++ b/drivers/net/ethernet/sfc/siena/siena.c @@ -943,6 +943,11 @@ static int siena_mtd_probe(struct efx_nic *efx) nvram_types >>= 1; } + if (n_parts == 0) { + kfree(parts); + return 0; + } + rc = siena_mtd_get_fw_subtypes(efx, parts, n_parts); if (rc) goto fail;
In some cases there is no mtd partitions that can be probed, so the mtd partitions list stays empty. This happens, for example, in SFC9220 devices on the second port of the NIC. The memory for the mtd partitions is deallocated in efx_mtd_remove, recovering the address of the first element of efx->mtd_list and then deallocating it. But if the list is empty, the address passed to kfree doesn't point to the memory allocated for the mtd partitions, but to the list head itself. Despite this hasn't caused other problems other than the memory leak, this is obviously incorrect. This patch deallocates the memory during mtd_probe in the case that there are no probed partitions, avoiding the leak. This was detected with kmemleak, output example: unreferenced object 0xffff88819cfa0000 (size 46560): comm "kworker/0:2", pid 48435, jiffies 4364987018 (age 45.924s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace: [<000000000f8e92d9>] kmalloc_order_trace+0x19/0x130 [<0000000042a03844>] efx_ef10_mtd_probe+0x12d/0x320 [sfc] [<000000004555654f>] efx_pci_probe.cold+0x4e1/0x6db [sfc] [<00000000b03d5126>] local_pci_probe+0xde/0x170 [<00000000376cc8d9>] work_for_cpu_fn+0x51/0xa0 [<00000000141f8de9>] process_one_work+0x8cb/0x1590 [<00000000cb2d8065>] worker_thread+0x707/0x1010 [<000000001ef4b9f6>] kthread+0x364/0x420 [<0000000014767137>] ret_from_fork+0x22/0x30 Fixes: 8127d661e77f ("sfc: Add support for Solarflare SFC9100 family") Reported-by: Liang Li <liali@redhat.com> Signed-off-by: Íñigo Huguet <ihuguet@redhat.com> --- drivers/net/ethernet/sfc/ef10.c | 5 +++++ drivers/net/ethernet/sfc/siena/siena.c | 5 +++++ 2 files changed, 10 insertions(+)