Re: [PATCH net 2/2] net: sfc: siena: fix memory leak in siena_mtd_probe()

[Taehee Yoo <ap420073@gmail.com>]

2022. 5. 11. 오후 3:25에 Martin Habets 이(가) 쓴 글:

Hi Martin,
Thanks a lot for your review!

 > On Tue, May 10, 2022 at 03:36:19PM +0000, Taehee Yoo wrote:
 >> In the NIC ->probe callback, ->mtd_probe() callback is called.
 >> If NIC has 2 ports, ->probe() is called twice and ->mtd_probe() too.
 >> In the ->mtd_probe(), which is siena_mtd_probe() it allocates and
 >> initializes mtd partiion.
 >> But mtd partition for sfc is shared data.
 >> So that allocated mtd partition data from last called
 >> siena_mtd_probe() will not be used.
 >
 > On Siena the 2nd port does have MTD partitions. In the output
 > from /proc/mtd below eth3 is the 1st port and eth4 is the 2nd
 > port:
 >
 > mtd12: 00030000 00010000 "eth3 sfc_mcfw:0b"
 > mtd13: 00010000 00010000 "eth3 sfc_dynamic_cfg:00"
 > mtd14: 00030000 00010000 "eth3 sfc_exp_rom:01"
 > mtd15: 00010000 00010000 "eth3 sfc_exp_rom_cfg:00"
 > mtd16: 00120000 00010000 "eth3 sfc_fpga:01"
 > mtd17: 00010000 00010000 "eth4 sfc_dynamic_cfg:00"
 > mtd18: 00010000 00010000 "eth4 sfc_exp_rom_cfg:00"
 >
 > So this patch is not needed, and efx_mtd_remove() will free
 > the memory for both ports.
 >

Okay, I will send a v2 patch tomorrow, that will drop this patch and 
unnecessary cover-letter.

Thanks!
Taehee Yoo

 > Martin
 >
 >> Therefore it must be freed.
 >> But it doesn't free a not used mtd partition data in siena_mtd_probe().
 >>
 >> Fixes: 8880f4ec21e6 ("sfc: Add support for SFC9000 family (2)")
 >> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
 >> ---
 >>   drivers/net/ethernet/sfc/siena.c | 5 +++++
 >>   1 file changed, 5 insertions(+)
 >>
 >> diff --git a/drivers/net/ethernet/sfc/siena.c 
b/drivers/net/ethernet/sfc/siena.c
 >> index ce3060e15b54..8b42951e34d6 100644
 >> --- a/drivers/net/ethernet/sfc/siena.c
 >> +++ b/drivers/net/ethernet/sfc/siena.c
 >> @@ -939,6 +939,11 @@ static int siena_mtd_probe(struct efx_nic *efx)
 >>   		nvram_types >>= 1;
 >>   	}
 >>
 >> +	if (!n_parts) {
 >> +		kfree(parts);
 >> +		return 0;
 >> +	}
 >> +
 >>   	rc = siena_mtd_get_fw_subtypes(efx, parts, n_parts);
 >>   	if (rc)
 >>   		goto fail;
 >> --
 >> 2.17.1