[PATCH 1/2] net: eth-uclass: Prevent data aborts with the Ethernet USB gadget

[Miquel Raynal]

From: Qianfan Zhao <qianfanguijin at 163.com>

The Ethernet USB gadget driver creates a virtual Ethernet interface on
the fly in the ->start() callback. This means, there is no other place
than the ->stop() callback to unregister the interface and free all the
associated structures. Deleting the interface frees the 'priv'
pointer. Unfortunately, the current code does not expect the 'priv'
pointer to disappear and tries to update its internal fields to mention
the interface is no longer running, which is obviously
wrong. Dereferencing a pointer and writing where it leads while the
buffer has been freed means we probably overwrite some heap data.

The fact is, this bug is reproducible since at least 2021, but the
occurrence of the data abort that is associated to it is changing over
the releases and the toolchains. This is probably due to the
use-after-free either touching a heap buffer or some heap meta-data. In
the former case, it does not trigger any failure, besides randomly
smashing some data. In the latter case, it totally breaks malloc
implementation which then processes totally broken data and likely
produces these data aborts.

On an am335x (Beagle Bone Black Wireless) with a GCC 10 toolchain, the
bug produces a data abort every 4 to 8 tftp downloads. With a GCC 11
toolchain, the crash happens at every try. In practice, all network
commands are affected, eg. dhcp and ping also lead to these data aborts,
which happen later in the code and are all pointing at "malloc".

Signed-off-by: Qianfan Zhao <qianfanguijin at 163.com>
[Miquel Raynal: Write a new commit message to argue why this fix is important]
Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>
---
 net/eth-uclass.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index b01a910938e..bfb092b86f9 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -354,8 +354,18 @@ void eth_halt(void)
 		return;
 
 	eth_get_ops(current)->stop(current);
-	priv->state = ETH_STATE_PASSIVE;
-	priv->running = false;
+
+	/* The Ethernet USB gadget driver creates a virtual Ethernet interface
+	 * on the fly in the ->start() callback, which means the ->stop()
+	 * callbacks deletes it. Deleting the interface frees the 'priv'
+	 * structure which shall no longer be dereferenced in this case. Ensure
+	 * 'priv' is still valid after ->stop(), before updating its fields.
+	 */
+	priv = dev_get_uclass_priv(current);
+	if (priv) {
+		priv->state = ETH_STATE_PASSIVE;
+		priv->running = false;
+	}
 }
 
 int eth_is_active(struct udevice *dev)
-- 
2.34.1