All network related commands are executed through a "net loop", which handles the initialization, setup and removal of all the required resources to perform the operation.
Aside from the generic network boilerplate, the net loop calls specific helpers provided by the different command implementations. The tftp implementation, among others, registers his own handler with net_set_udp_handler() in order to catch incoming UDP packets. In this handler, there are several calls to eth_halt(). Some of them might be legitimate, but one that is certainly not is the one in the switch case handling the "access denied" and "file not found errors". When these error happens, there is nothing wrong with the network interface but the protocol itself returned an error condition. There is no reason to immediately halt the Ethernet interface, because the handler will propagate an error code which the net loop will process, up to the point where it will halt and remove the Ethernet interface itself. Said otherwise, halting the Ethernet interface (and at the same time removing all structures related to it) in the tftp handler bypasses the net loop. This would not cause any harm if halting the interface would not sometimes lead to its entire removal. That is indeed what happens with the Ethernet USB gadget implementation. In this driver, the ->start() and ->stop() callbacks are responsible for creating and removing a virtual Ethernet interface, which means after the ->stop() call, any structure used to define that interface shall no longer be used. The eth_halt() call stopping (and removing) the interface in the handler, would make the net loop perform illegal accesses over freed buffers, overwriting some content in the heap. These overwrites have been identified to later lead to serious data aborts within malloc() execution. Let's just drop this eth_alt() call which is happening too early, and is redundant anyway. Suggested-by: Qianfan Zhao <[email protected]> Signed-off-by: Miquel Raynal <[email protected]> --- net/tftp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/net/tftp.c b/net/tftp.c index 88e71e67de3..ac20a60d222 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -680,7 +680,6 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip, case TFTP_ERR_FILE_NOT_FOUND: case TFTP_ERR_ACCESS_DENIED: puts("Not retrying...\n"); - eth_halt(); net_set_state(NETLOOP_FAIL); break; case TFTP_ERR_UNDEFINED: -- 2.34.1
