[PATCH v3 2/2] efi_loader: fix use after free in receive path
Patrick Wildt
patrick at blueri.se
Wed Oct 7 11:04:33 CEST 2020
With DM enabled the ethernet code will receive a packet, call
the push method that's set by the EFI network implementation
and then free the packet. Unfortunately the push methods only
sets a flag that the packet needs to be handled, but the code
that provides the packet to an EFI application runs after the
packet has already been freed.
To rectify this issue, adjust the push method to accept the packet
and store it in a temporary buffer. The EFI application then gets
the data copied from that buffer. This way the packet is cached
until is is needed.
The DM Ethernet stack tries to receive 32 packets at once, thus
we better allocate as many buffers as the stack.
Signed-off-by: Patrick Wildt <patrick at blueri.se>
---
Changes from v2:
- Inverted the logic of efi_net_receive() reflecting the fill level,
since in v2 it was the wrong way around.
- Clearing the packet queue by simply setting the amount of packets
in the queue to zero.
Changes from v1:
- Reimplemented the buffer handling approach to use pre-allocated
buffers.
lib/efi_loader/efi_net.c | 92 ++++++++++++++++++++++++++++++----------
1 file changed, 69 insertions(+), 23 deletions(-)
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index 22f0123eca..69276b275d 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -24,9 +24,12 @@ static const efi_guid_t efi_net_guid = EFI_SIMPLE_NETWORK_PROTOCOL_GUID;
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 void *new_tx_packet;
static void *transmit_buffer;
+static uchar **receive_buffer;
+static size_t *receive_lengths;
+static int rx_packet_idx;
+static int rx_packet_num;
/*
* The notification function of this event is called in every timer cycle
@@ -115,6 +118,8 @@ static efi_status_t EFIAPI efi_net_stop(struct efi_simple_network *this)
} else {
/* Disable hardware and put it into the reset state */
eth_halt();
+ /* Clear cache of packets */
+ rx_packet_num = 0;
this->mode->state = EFI_NETWORK_STOPPED;
}
out:
@@ -160,6 +165,8 @@ static efi_status_t EFIAPI efi_net_initialize(struct efi_simple_network *this,
net_init();
/* Disable hardware and put it into the reset state */
eth_halt();
+ /* Clear cache of packets */
+ rx_packet_num = 0;
/* Set current device according to environment variables */
eth_set_current();
/* Get hardware ready for send and receive operations */
@@ -602,16 +609,16 @@ static efi_status_t EFIAPI efi_net_receive
break;
}
- if (!new_rx_packet) {
+ if (!rx_packet_num) {
ret = EFI_NOT_READY;
goto out;
}
/* Fill export parameters */
- eth_hdr = (struct ethernet_hdr *)net_rx_packet;
+ eth_hdr = (struct ethernet_hdr *)receive_buffer[rx_packet_idx];
protlen = ntohs(eth_hdr->et_protlen);
if (protlen == 0x8100) {
hdr_size += 4;
- protlen = ntohs(*(u16 *)&net_rx_packet[hdr_size - 2]);
+ protlen = ntohs(*(u16 *)&receive_buffer[rx_packet_idx][hdr_size - 2]);
}
if (header_size)
*header_size = hdr_size;
@@ -621,17 +628,22 @@ 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 < receive_lengths[rx_packet_idx]) {
/* Packet doesn't fit, try again with bigger buffer */
- *buffer_size = net_rx_packet_len;
+ *buffer_size = receive_lengths[rx_packet_idx];
ret = EFI_BUFFER_TOO_SMALL;
goto out;
}
/* Copy packet */
- memcpy(buffer, net_rx_packet, net_rx_packet_len);
- *buffer_size = net_rx_packet_len;
- new_rx_packet = 0;
- this->int_status &= ~EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
+ memcpy(buffer, receive_buffer[rx_packet_idx],
+ receive_lengths[rx_packet_idx]);
+ *buffer_size = receive_lengths[rx_packet_idx];
+ rx_packet_idx = (rx_packet_idx + 1) % ETH_PACKETS_BATCH_RECV;
+ rx_packet_num--;
+ if (rx_packet_num)
+ wait_for_packet->is_signaled = true;
+ else
+ this->int_status &= ~EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
out:
return EFI_EXIT(ret);
}
@@ -664,7 +676,26 @@ void efi_net_set_dhcp_ack(void *pkt, int len)
*/
static void efi_net_push(void *pkt, int len)
{
- new_rx_packet = true;
+ int rx_packet_next;
+
+ /* Check that we at least received an Ethernet header */
+ if (len < sizeof(struct ethernet_hdr))
+ return;
+
+ /* Check that the buffer won't overflow */
+ if (len > PKTSIZE_ALIGN)
+ return;
+
+ /* Can't store more than pre-alloced buffer */
+ if (rx_packet_num >= ETH_PACKETS_BATCH_RECV)
+ return;
+
+ rx_packet_next = (rx_packet_idx + rx_packet_num) %
+ ETH_PACKETS_BATCH_RECV;
+ memcpy(receive_buffer[rx_packet_next], pkt, len);
+ receive_lengths[rx_packet_next] = len;
+
+ rx_packet_num++;
}
/**
@@ -689,20 +720,14 @@ static void EFIAPI efi_network_timer_notify(struct efi_event *event,
if (!this || this->mode->state != EFI_NETWORK_INITIALIZED)
goto out;
- if (!new_rx_packet) {
+ if (!rx_packet_num) {
push_packet = efi_net_push;
eth_rx();
push_packet = NULL;
- if (new_rx_packet) {
- /* Check that we at least received an Ethernet header */
- if (net_rx_packet_len >=
- sizeof(struct ethernet_hdr)) {
- this->int_status |=
- EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
- wait_for_packet->is_signaled = true;
- } else {
- new_rx_packet = 0;
- }
+ if (rx_packet_num) {
+ this->int_status |=
+ EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT;
+ wait_for_packet->is_signaled = true;
}
}
out:
@@ -830,6 +855,7 @@ efi_status_t efi_net_register(void)
{
struct efi_net_obj *netobj = NULL;
efi_status_t r;
+ int i;
if (!eth_get_dev()) {
/* No network device active, don't expose any */
@@ -847,6 +873,21 @@ efi_status_t efi_net_register(void)
goto out_of_resources;
transmit_buffer = (void *)ALIGN((uintptr_t)transmit_buffer, PKTALIGN);
+ /* Allocate a number of receive buffers */
+ receive_buffer = calloc(ETH_PACKETS_BATCH_RECV,
+ sizeof(*receive_buffer));
+ if (!receive_buffer)
+ goto out_of_resources;
+ for (i = 0; i < ETH_PACKETS_BATCH_RECV; i++) {
+ receive_buffer[i] = malloc(PKTSIZE_ALIGN);
+ if (!receive_buffer[i])
+ goto out_of_resources;
+ }
+ receive_lengths = calloc(ETH_PACKETS_BATCH_RECV,
+ sizeof(*receive_lengths));
+ if (!receive_lengths)
+ goto out_of_resources;
+
/* Hook net up to the device list */
efi_add_handle(&netobj->header);
@@ -941,7 +982,12 @@ failure_to_add_protocol:
return r;
out_of_resources:
free(netobj);
- /* free(transmit_buffer) not needed yet */
+ free(transmit_buffer);
+ if (receive_buffer)
+ for (i = 0; i < ETH_PACKETS_BATCH_RECV; i++)
+ free(receive_buffer[i]);
+ free(receive_buffer);
+ free(receive_lengths);
printf("ERROR: Out of memory\n");
return EFI_OUT_OF_RESOURCES;
}
--
2.28.0
More information about the U-Boot
mailing list