| Message ID | 7adc1815237605a0b774efb31a2ab22df51462d3.1620890610.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e1d027dd97e1e750669cdc0d3b016a4f54e473eb |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: mdio: octeon: Fix some double free issues | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: 'Suggested-by:' is the preferred signature form |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Thu, May 13, 2021 at 09:24:55AM +0200, Christophe JAILLET wrote: > 'bus->mii_bus' has been allocated with 'devm_mdiobus_alloc_size()' in the > probe function. So it must not be freed explicitly or there will be a > double free. > > Remove the incorrect 'mdiobus_free' in the error handling path of the > probe function and in remove function. > > Suggested-By: Andrew Lunn <andrew@lunn.ch> > Fixes: 35d2aeac9810 ("phy: mdio-octeon: Use devm_mdiobus_alloc_size()") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Reviewed-by: Russell King <rmk+kernel@armlinux.org.uk> > --- > The 'smi_en.u64 = 0; oct_mdio_writeq()' looks odd to me. Usually the normal > path and the error handling path don't write the same value. Here, both > write 0. > Having '1' somewhere would 'look' more usual. :) > More over I think that 'smi_en.s.en = 1;' in the probe is useless. It looks fine to me. smi_en.u64 = 0; smi_en.s.en = 1; oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); smi_en is a union of a u64 and a structure containing a bitfield. s.en corresponds on LE systems with the u64 bit 0. So the above has the effect of writing a u64 value of '1' to the SMI_EN register, whereas: smi_en.u64 = 0; oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); has the effect of writing a u64 value of '0' to the SMI_EN register. This code is fine.
On Thu, May 13, 2021 at 09:24:55AM +0200, Christophe JAILLET wrote: > 'bus->mii_bus' has been allocated with 'devm_mdiobus_alloc_size()' in the > probe function. So it must not be freed explicitly or there will be a > double free. > > Remove the incorrect 'mdiobus_free' in the error handling path of the > probe function and in remove function. > > Suggested-By: Andrew Lunn <andrew@lunn.ch> > Fixes: 35d2aeac9810 ("phy: mdio-octeon: Use devm_mdiobus_alloc_size()") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Thu, 13 May 2021 09:24:55 +0200 you wrote: > 'bus->mii_bus' has been allocated with 'devm_mdiobus_alloc_size()' in the > probe function. So it must not be freed explicitly or there will be a > double free. > > Remove the incorrect 'mdiobus_free' in the error handling path of the > probe function and in remove function. > > [...] Here is the summary with links: - net: mdio: octeon: Fix some double free issues https://git.kernel.org/netdev/net/c/e1d027dd97e1 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/drivers/net/mdio/mdio-octeon.c b/drivers/net/mdio/mdio-octeon.c index 8ce99c4888e1..e096e68ac667 100644 --- a/drivers/net/mdio/mdio-octeon.c +++ b/drivers/net/mdio/mdio-octeon.c @@ -71,7 +71,6 @@ static int octeon_mdiobus_probe(struct platform_device *pdev) return 0; fail_register: - mdiobus_free(bus->mii_bus); smi_en.u64 = 0; oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); return err; @@ -85,7 +84,6 @@ static int octeon_mdiobus_remove(struct platform_device *pdev) bus = platform_get_drvdata(pdev); mdiobus_unregister(bus->mii_bus); - mdiobus_free(bus->mii_bus); smi_en.u64 = 0; oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN); return 0;
'bus->mii_bus' has been allocated with 'devm_mdiobus_alloc_size()' in the probe function. So it must not be freed explicitly or there will be a double free. Remove the incorrect 'mdiobus_free' in the error handling path of the probe function and in remove function. Suggested-By: Andrew Lunn <andrew@lunn.ch> Fixes: 35d2aeac9810 ("phy: mdio-octeon: Use devm_mdiobus_alloc_size()") Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- The 'smi_en.u64 = 0; oct_mdio_writeq()' looks odd to me. Usually the normal path and the error handling path don't write the same value. Here, both write 0. Having '1' somewhere would 'look' more usual. :) More over I think that 'smi_en.s.en = 1;' in the probe is useless. I don't know what this code is supposed to do, so I leave it as-is. But it looks like a left-over from a6d678645210. --- drivers/net/mdio/mdio-octeon.c | 2 -- 1 file changed, 2 deletions(-)