| Message ID | de7fe2da7ae4a85252d63904fae61585e4641936.1714117337.git.matthias.schiffer@ew.tq-group.com |
|---|---|
| State | Accepted |
| Commit | ef9d4da6c92b07f0668f059d0cdc51eb34ed947e |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/5] net: eth-uclass: guard against reentrant eth_init()/eth_halt() calls | expand |
On Fri, Apr 26, 2024 at 10:02:24AM +0200, Matthias Schiffer wrote: > With netconsole, any log message can result in an eth_init(), possibly > causing an reentrant call into eth_init() if a driver's ops print > anything: > > eth_init() -> driver.start() -> printf() -> netconsole -> eth_init() > eth_halt() -> driver.stop() -> printf() -> netconsole -> eth_init() > > Rather than expecting every single Ethernet driver to handle this case, > prevent the reentrant calls in eth_init() and eth_halt(). > > The issue was noticed on an AM62x board, where a bootm after > simultaneous netconsole and TFTP would result in a crash. Link: https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1350550/sk-am62a-lp-am65_cpsw_nuss_port-error-in-u-boot-while-using-netconsole-functionality/ > > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > --- > net/eth-uclass.c | 40 ++++++++++++++++++++++++++++++++-------- > 1 file changed, 32 insertions(+), 8 deletions(-) > > diff --git a/net/eth-uclass.c b/net/eth-uclass.c > index 3d0ec91dfa4..193218a328b 100644 > --- a/net/eth-uclass.c > +++ b/net/eth-uclass.c > @@ -48,6 +48,8 @@ struct eth_uclass_priv { > > /* eth_errno - This stores the most recent failure code from DM functions */ > static int eth_errno; > +/* Are we currently in eth_init() or eth_halt()? */ > +static bool in_init_halt; > > /* board-specific Ethernet Interface initializations. */ > __weak int board_interface_eth_init(struct udevice *dev, > @@ -285,11 +287,19 @@ U_BOOT_ENV_CALLBACK(ethaddr, on_ethaddr); > > int eth_init(void) > { > - char *ethact = env_get("ethact"); > - char *ethrotate = env_get("ethrotate"); > struct udevice *current = NULL; > struct udevice *old_current; > int ret = -ENODEV; > + char *ethrotate; > + char *ethact; > + > + if (in_init_halt) > + return -EBUSY; > + > + in_init_halt = true; > + > + ethact = env_get("ethact"); > + ethrotate = env_get("ethrotate"); > > /* > * When 'ethrotate' variable is set to 'no' and 'ethact' variable > @@ -298,8 +308,10 @@ int eth_init(void) > if ((ethrotate != NULL) && (strcmp(ethrotate, "no") == 0)) { > if (ethact) { > current = eth_get_dev_by_name(ethact); > - if (!current) > - return -EINVAL; > + if (!current) { > + ret = -EINVAL; > + goto end; > + } > } > } > > @@ -307,7 +319,8 @@ int eth_init(void) > current = eth_get_dev(); > if (!current) { > log_err("No ethernet found.\n"); > - return -ENODEV; > + ret = -ENODEV; > + goto end; > } > } > > @@ -324,7 +337,8 @@ int eth_init(void) > > priv->state = ETH_STATE_ACTIVE; > priv->running = true; > - return 0; > + ret = 0; > + goto end; > } > } else { > ret = eth_errno; > @@ -344,6 +358,8 @@ int eth_init(void) > current = eth_get_dev(); > } while (old_current != current); > > +end: > + in_init_halt = false; > return ret; > } > > @@ -352,17 +368,25 @@ void eth_halt(void) > struct udevice *current; > struct eth_device_priv *priv; > > + if (in_init_halt) > + return; > + > + in_init_halt = true; > + > current = eth_get_dev(); > if (!current) > - return; > + goto end; > > priv = dev_get_uclass_priv(current); > if (!priv || !priv->running) > - return; > + goto end; > > eth_get_ops(current)->stop(current); > priv->state = ETH_STATE_PASSIVE; > priv->running = false; > + > +end: > + in_init_halt = false; > } > > int eth_is_active(struct udevice *dev) > -- > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > Amtsgericht München, HRB 105018 > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > https://www.tq-group.com/ >
On Fri, 26 Apr 2024 10:02:24 +0200, Matthias Schiffer wrote: > With netconsole, any log message can result in an eth_init(), possibly > causing an reentrant call into eth_init() if a driver's ops print > anything: > > eth_init() -> driver.start() -> printf() -> netconsole -> eth_init() > eth_halt() -> driver.stop() -> printf() -> netconsole -> eth_init() > > [...] Applied to u-boot/next, thanks!
diff --git a/net/eth-uclass.c b/net/eth-uclass.c index 3d0ec91dfa4..193218a328b 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -48,6 +48,8 @@ struct eth_uclass_priv { /* eth_errno - This stores the most recent failure code from DM functions */ static int eth_errno; +/* Are we currently in eth_init() or eth_halt()? */ +static bool in_init_halt; /* board-specific Ethernet Interface initializations. */ __weak int board_interface_eth_init(struct udevice *dev, @@ -285,11 +287,19 @@ U_BOOT_ENV_CALLBACK(ethaddr, on_ethaddr); int eth_init(void) { - char *ethact = env_get("ethact"); - char *ethrotate = env_get("ethrotate"); struct udevice *current = NULL; struct udevice *old_current; int ret = -ENODEV; + char *ethrotate; + char *ethact; + + if (in_init_halt) + return -EBUSY; + + in_init_halt = true; + + ethact = env_get("ethact"); + ethrotate = env_get("ethrotate"); /* * When 'ethrotate' variable is set to 'no' and 'ethact' variable @@ -298,8 +308,10 @@ int eth_init(void) if ((ethrotate != NULL) && (strcmp(ethrotate, "no") == 0)) { if (ethact) { current = eth_get_dev_by_name(ethact); - if (!current) - return -EINVAL; + if (!current) { + ret = -EINVAL; + goto end; + } } } @@ -307,7 +319,8 @@ int eth_init(void) current = eth_get_dev(); if (!current) { log_err("No ethernet found.\n"); - return -ENODEV; + ret = -ENODEV; + goto end; } } @@ -324,7 +337,8 @@ int eth_init(void) priv->state = ETH_STATE_ACTIVE; priv->running = true; - return 0; + ret = 0; + goto end; } } else { ret = eth_errno; @@ -344,6 +358,8 @@ int eth_init(void) current = eth_get_dev(); } while (old_current != current); +end: + in_init_halt = false; return ret; } @@ -352,17 +368,25 @@ void eth_halt(void) struct udevice *current; struct eth_device_priv *priv; + if (in_init_halt) + return; + + in_init_halt = true; + current = eth_get_dev(); if (!current) - return; + goto end; priv = dev_get_uclass_priv(current); if (!priv || !priv->running) - return; + goto end; eth_get_ops(current)->stop(current); priv->state = ETH_STATE_PASSIVE; priv->running = false; + +end: + in_init_halt = false; } int eth_is_active(struct udevice *dev)
With netconsole, any log message can result in an eth_init(), possibly causing an reentrant call into eth_init() if a driver's ops print anything: eth_init() -> driver.start() -> printf() -> netconsole -> eth_init() eth_halt() -> driver.stop() -> printf() -> netconsole -> eth_init() Rather than expecting every single Ethernet driver to handle this case, prevent the reentrant calls in eth_init() and eth_halt(). The issue was noticed on an AM62x board, where a bootm after simultaneous netconsole and TFTP would result in a crash. Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> --- net/eth-uclass.c | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-)