efi_loader: fix free() before use in RX path

[Patrick Wildt]

Hi,

I have an EFI application doing TFTP on an i.MX8MM board.  The EFI
application uses the Simple Network Protocol and does ARP itself.
ARP didn't work, and I saw that the replies that were received on
the board were broken.

Good, as seen from the sending host:
2acbc7b16422001999ed548808060001080006040002001999ed5488ac1f00012acbc7b16422ac1f007f000000000000000000000000000000000000

Bad, as seen on the i.MX8MM board:
60a7fc7f0000000060a7fc7f00000000080006040002001999ed5488ac1f00012acbc7b16422ac1f007f000000000000000000000000000000000000

As you can see, in the received packet, it looks like the the first
16 bytes were overwritten, and those look like two 64-bit pointer.

Looking through the stack, with uclass enabled, the code does:

eth_rx():
[..]
    ret = eth_get_ops(current)->recv(current, flags, &packet);
    flags = 0;
    if (ret > 0)
            net_process_received_packet(packet, ret);
    if (ret >= 0 && eth_get_ops(current)->free_pkt)
            eth_get_ops(current)->free_pkt(current, packet, ret);
[..]

recv returns the packet, free_pkt free()s the packet.  Thus we may
only use the packet's contents between the recv and the free_pkt.

net_process_received_packet():
[..]
    net_rx_packet = in_packet;
    net_rx_packet_len = len;
[..]
    if (push_packet) {
        (*push_packet)(in_packet, len);
        return;
   }
[..]

push_packet is set to efi_net_push during efi_network_timer_notify,
which does nothing more than to set a status flag:

static void efi_net_push(void *pkt, int len)
{
    new_rx_packet = true;
}

This does *not* touch the packet at all.  Instead, efi_net_receive
will, as soon as the EFI application calls it, access net_rx_packet
directly.  But this only happens *after* the packet has already been
free()d.  Something else could have already started using that memory!

The following diff changes efi_net_push() to allocate a new buffer, but
I think it's not enough since eth_rx() will try to receive 32 packets
at one time.  So I think maybe efi_net_push() needs to add the packets
to a list, and efi_net_receive() takes the first packet from that list.

Best regards,
Patrick

Signed-off-by: Patrick Wildt <patrick at blueri.se>

diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index 22f0123eca..6e3c29cba2 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -25,6 +25,8 @@ static const efi_guid_t efi_pxe_base_code_protocol_guid =
 					EFI_PXE_BASE_CODE_PROTOCOL_GUID;
 static struct efi_pxe_packet *dhcp_ack;
 static bool new_rx_packet;
+static uchar *efi_net_rx_packet;
+static int efi_net_rx_packet_len;
 static void *new_tx_packet;
 static void *transmit_buffer;
 
@@ -607,11 +609,11 @@ static efi_status_t EFIAPI efi_net_receive
 		goto out;
 	}
 	/* Fill export parameters */
-	eth_hdr = (struct ethernet_hdr *)net_rx_packet;
+	eth_hdr = (struct ethernet_hdr *)efi_net_rx_packet;
 	protlen = ntohs(eth_hdr->et_protlen);
 	if (protlen == 0x8100) {
 		hdr_size += 4;
-		protlen = ntohs(*(u16 *)&net_rx_packet[hdr_size - 2]);
+		protlen = ntohs(*(u16 *)&efi_net_rx_packet[hdr_size - 2]);
 	}
 	if (header_size)
 		*header_size = hdr_size;
@@ -621,16 +623,19 @@ static efi_status_t EFIAPI efi_net_receive
 		memcpy(src_addr, eth_hdr->et_src, ARP_HLEN);
 	if (protocol)
 		*protocol = protlen;
-	if (*buffer_size < net_rx_packet_len) {
+	if (*buffer_size < efi_net_rx_packet_len) {
 		/* Packet doesn't fit, try again with bigger buffer */
-		*buffer_size = net_rx_packet_len;
+		*buffer_size = efi_net_rx_packet_len;
 		ret = EFI_BUFFER_TOO_SMALL;
 		goto out;
 	}
 	/* Copy packet */
-	memcpy(buffer, net_rx_packet, net_rx_packet_len);
-	*buffer_size = net_rx_packet_len;
+	memcpy(buffer, efi_net_rx_packet, efi_net_rx_packet_len);
+	*buffer_size = efi_net_rx_packet_len;
 	new_rx_packet = 0;
+	free(efi_net_rx_packet);
+	efi_net_rx_packet = NULL;
+	efi_net_rx_packet_len = 0;
 	this->int_status &= ~EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
 out:
 	return EFI_EXIT(ret);
@@ -664,6 +669,13 @@ void efi_net_set_dhcp_ack(void *pkt, int len)
  */
 static void efi_net_push(void *pkt, int len)
 {
+	efi_net_rx_packet = malloc(len);
+	if (efi_net_rx_packet == NULL)
+		return;
+
+	memcpy(efi_net_rx_packet, pkt, len);
+	efi_net_rx_packet_len = len;
+
 	new_rx_packet = true;
 }