Messages in this thread | | | From | "Cai,Huoqing" <> | Subject | RE: [PATCH] net: arc_emac: Fix use after free in arc_mdio_probe() | Date | Tue, 8 Mar 2022 13:50:40 +0000 |
| |
Hello,
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: 2022年3月8日 21:43 > To: Jianglei Nie > Cc: davem@davemloft.net; kuba@kernel.org; Cai,Huoqing; > netdev@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] net: arc_emac: Fix use after free in arc_mdio_probe() If resend a patch, you can use prefix "[PATCH v2]" in subject. e.g. git format-patch -1 -v2 > > On Tue, Mar 08, 2022 at 07:10:05PM +0800, Jianglei Nie wrote: > > If bus->state is equal to MDIOBUS_ALLOCATED, mdiobus_free(bus) will > free > > the "bus". But bus->name is still used in the next line, which will lead > > to a use after free. > > > > We can fix it by putting the bus->name in a local variable and then use > > the name in the error message without referring to bus to avoid the uaf. > > > > Signed-off-by: Jianglei Nie <niejianglei2021@163.com> > > --- > > drivers/net/ethernet/arc/emac_mdio.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/ethernet/arc/emac_mdio.c > b/drivers/net/ethernet/arc/emac_mdio.c > > index 9acf589b1178..33fd63d227ef 100644 > > --- a/drivers/net/ethernet/arc/emac_mdio.c > > +++ b/drivers/net/ethernet/arc/emac_mdio.c > > @@ -134,6 +134,7 @@ int arc_mdio_probe(struct arc_emac_priv *priv) > > struct device_node *np = priv->dev->of_node; > > struct mii_bus *bus; > > int error; > > + const char *name = "Synopsys MII Bus"; > > Netdev uses reverse christmass tree, meaning you need to sort > variables longest to shortest. > > I'm also wondering about the lifetime of name. name itself is a stack > variable, so it will disappear as soon as the function exits. The > string itself is in the rodata section. But is a copy made onto the > stack, or does bus->name point to the rodata? > > Andrew > > > bus = mdiobus_alloc(); > > if (!bus) > > @@ -142,7 +143,7 @@ int arc_mdio_probe(struct arc_emac_priv *priv) > > priv->bus = bus; > > bus->priv = priv; > > bus->parent = priv->dev; > > - bus->name = "Synopsys MII Bus"; > > + bus->name = name; > > bus->read = &arc_mdio_read; > > bus->write = &arc_mdio_write; > > bus->reset = &arc_mdio_reset; > > @@ -167,7 +168,7 @@ int arc_mdio_probe(struct arc_emac_priv *priv) > > if (error) { > > mdiobus_free(bus); > > return dev_err_probe(priv->dev, error, > > - "cannot register MDIO bus %s\n", bus- > >name); > > + "cannot register MDIO bus %s\n", name); > > } > > > > return 0; > > -- > > 2.25.1 > >
| |