| Message ID | 20241215112417.2854371-1-joe@pf.is.s.u-tokyo.ac.jp (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] net: mdiobus: fix an OF node reference leak | expand |
On Sun, Dec 15, 2024 at 08:24:17PM +0900, Joe Hattori wrote: > fwnode_find_mii_timestamper() calls of_parse_phandle_with_fixed_args() > but does not decrement the refcount of the obtained OF node. Add an > of_node_put() call before returning from the function. > > This bug was detected by an experimental static analysis tool that I am > developing. > > Fixes: bc1bee3b87ee ("net: mdiobus: Introduce fwnode_mdiobus_register_phy()") > Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp> > --- > Changes in v2: > - Call of_node_put() after calling register_mii_timestamper() to avoid > UAF. > --- > drivers/net/mdio/fwnode_mdio.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c > index b156493d7084..456f829e4d6d 100644 > --- a/drivers/net/mdio/fwnode_mdio.c > +++ b/drivers/net/mdio/fwnode_mdio.c > @@ -41,6 +41,7 @@ static struct mii_timestamper * > fwnode_find_mii_timestamper(struct fwnode_handle *fwnode) > { > struct of_phandle_args arg; > + struct mii_timestamper *mii_ts; > int err; > > if (is_acpi_node(fwnode)) > @@ -56,7 +57,9 @@ fwnode_find_mii_timestamper(struct fwnode_handle *fwnode) > if (arg.args_count != 1) > return ERR_PTR(-EINVAL); Is there no need to put the node when arg.args_count != 1 ? Andrew
On 12/16/24 01:06, Andrew Lunn wrote: > On Sun, Dec 15, 2024 at 08:24:17PM +0900, Joe Hattori wrote: >> fwnode_find_mii_timestamper() calls of_parse_phandle_with_fixed_args() >> but does not decrement the refcount of the obtained OF node. Add an >> of_node_put() call before returning from the function. >> >> This bug was detected by an experimental static analysis tool that I am >> developing. >> >> Fixes: bc1bee3b87ee ("net: mdiobus: Introduce fwnode_mdiobus_register_phy()") >> Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp> >> --- >> Changes in v2: >> - Call of_node_put() after calling register_mii_timestamper() to avoid >> UAF. >> --- >> drivers/net/mdio/fwnode_mdio.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c >> index b156493d7084..456f829e4d6d 100644 >> --- a/drivers/net/mdio/fwnode_mdio.c >> +++ b/drivers/net/mdio/fwnode_mdio.c >> @@ -41,6 +41,7 @@ static struct mii_timestamper * >> fwnode_find_mii_timestamper(struct fwnode_handle *fwnode) >> { >> struct of_phandle_args arg; >> + struct mii_timestamper *mii_ts; >> int err; >> >> if (is_acpi_node(fwnode)) >> @@ -56,7 +57,9 @@ fwnode_find_mii_timestamper(struct fwnode_handle *fwnode) >> if (arg.args_count != 1) >> return ERR_PTR(-EINVAL); > > Is there no need to put the node when arg.args_count != 1 ? Yes, should have caught that. Fixed in the v3 patch. > > Andrew Best, Joe
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c index b156493d7084..456f829e4d6d 100644 --- a/drivers/net/mdio/fwnode_mdio.c +++ b/drivers/net/mdio/fwnode_mdio.c @@ -41,6 +41,7 @@ static struct mii_timestamper * fwnode_find_mii_timestamper(struct fwnode_handle *fwnode) { struct of_phandle_args arg; + struct mii_timestamper *mii_ts; int err; if (is_acpi_node(fwnode)) @@ -56,7 +57,9 @@ fwnode_find_mii_timestamper(struct fwnode_handle *fwnode) if (arg.args_count != 1) return ERR_PTR(-EINVAL); - return register_mii_timestamper(arg.np, arg.args[0]); + mii_ts = register_mii_timestamper(arg.np, arg.args[0]); + of_node_put(arg.np); + return mii_ts; } int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
fwnode_find_mii_timestamper() calls of_parse_phandle_with_fixed_args() but does not decrement the refcount of the obtained OF node. Add an of_node_put() call before returning from the function. This bug was detected by an experimental static analysis tool that I am developing. Fixes: bc1bee3b87ee ("net: mdiobus: Introduce fwnode_mdiobus_register_phy()") Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp> --- Changes in v2: - Call of_node_put() after calling register_mii_timestamper() to avoid UAF. --- drivers/net/mdio/fwnode_mdio.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)