| Message ID | 20220510153619.32464-3-ap420073@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: sfc: fix memory leak in ->mtd_probe() callback | expand |
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. 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
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
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;
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. 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(+)