On 12/12/22 16:29, Niel Fourie wrote:
In eth_halt(), reread and revalidate priv after calling stop(), as it may have been freed, leaving a dangling pointer.
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
Changes for v2:
- Revalidate priv instead of changing state before stop()
- Added explanational comment
This patch my be dropped if the patch which addresses the root cause ("usb: gadget: ether: split start/stop from init/halt") is accepted.
net/eth-uclass.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/eth-uclass.c b/net/eth-uclass.c index f41da4b37b3..7d5783b5cab 100644 --- a/net/eth-uclass.c +++ b/net/eth-uclass.c @@ -341,8 +341,11 @@ void eth_halt(void) priv = dev_get_uclass_priv(current); if (!priv || !priv->running) return;
This shouldn't be here.
eth_get_ops(current)->stop(current);
- /* Ethernet gadget frees priv during stop, workaround until fixed... */
- priv = dev_get_uclass_priv(current);
- if (!priv || !priv->running)
return;
My understanding of discussion in [PATCH] usb: gadget: ether: split start/stop from init/halt is that this patch is not needed with the aforementioned one applied.