Re: [PATCH] net: eth-uclass: change state before stop() in eth_halt()

[Lukasz Majewski <lukma@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 1650 bytes --]

On Wed, 30 Nov 2022 17:42:25 +0100
Niel Fourie <lusus@denx.de> wrote:

> In eth_halt(), change the private uclass state before calling
> stop() instead of afterwards, to avoid writing to memory which
> may have been freed during stop().
> 
> In the ethernet gadget implementation, the gadget device gets
> probed during start() and removed during stop(), which includes
> freeing `uclass_priv_` to which `priv` is pointing. Writing to
> `priv` after stop() may corrupt the `fd` member of `struct
> malloc_chunk`, which represents the freed block, and could cause
> hard-to-debug crashes on subsequent calls to malloc()/free().
> 
> Signed-off-by: Niel Fourie <lusus@denx.de>
> Cc: Ramon Fried <rfried.dev@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Lukasz Majewski <lukma@denx.de>
> ---
>  net/eth-uclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/eth-uclass.c b/net/eth-uclass.c
> index f41da4b37b3..bc3b9751e32 100644
> --- a/net/eth-uclass.c
> +++ b/net/eth-uclass.c
> @@ -342,9 +342,9 @@ void eth_halt(void)
>  	if (!priv || !priv->running)
>  		return;
>  
> -	eth_get_ops(current)->stop(current);
>  	priv->state = ETH_STATE_PASSIVE;
>  	priv->running = false;
> +	eth_get_ops(current)->stop(current);
>  }
>  
>  int eth_is_active(struct udevice *dev)

Reviewed-by: Lukasz Majewski <lukma@denx.de>


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]