Re: [PATCH 1/1] efi_loader: fix handling of DHCP acknowledge

[Heinrich Schuchardt]

On 11/30/22 08:37, Ilias Apalodimas wrote:

Hi Heinrich,

On Sat, Nov 26, 2022 at 05:10:56PM +0100, Heinrich Schuchardt wrote:

The dhcp command may be executed after the first UEFI command.
We should still update the EFI_PXE_BASE_CODE_PROTOCOL.

Don't leak content of prior acknowledge packages.

Handle out of memory.

The patch looks correct, but the description is a bit confusing.  Apart
from what you mention this patch also fixes

- An unchecked allocation

I can add this.

- shadowing of the global netobj

netobj was a local variable before this patch.

Thanks for reviewing.

Best regards

Heinrich

Can we please update the commit message to something that mentions all of
these?

Thanks
/Ilias

Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
---
  lib/efi_loader/efi_net.c | 13 ++++++++++---
  1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index 69276b275d..96a5bcca27 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -30,6 +30,7 @@ static uchar **receive_buffer;
  static size_t *receive_lengths;
  static int rx_packet_idx;
  static int rx_packet_num;
+static struct efi_net_obj *netobj;

/*

   * The notification function of this event is called in every timer cycle
@@ -660,10 +661,16 @@ void efi_net_set_dhcp_ack(void *pkt, int len)
  {
        int maxsize = sizeof(*dhcp_ack);

- if (!dhcp_ack)

+       if (!dhcp_ack) {
                dhcp_ack = malloc(maxsize);
-
+               if (!dhcp_ack)
+                       return;
+       }
+       memset(dhcp_ack, 0, maxsize);
        memcpy(dhcp_ack, pkt, min(len, maxsize));
+
+       if (netobj)
+               netobj->pxe_mode.dhcp_ack = *dhcp_ack;
  }

/**

@@ -853,7 +860,6 @@ static efi_status_t EFIAPI efi_pxe_base_code_set_packets(
   */
  efi_status_t efi_net_register(void)
  {
-       struct efi_net_obj *netobj = NULL;
        efi_status_t r;
        int i;

@@ -982,6 +988,7 @@ failure_to_add_protocol:

        return r;
  out_of_resources:
        free(netobj);
+       netobj = NULL;
        free(transmit_buffer);
        if (receive_buffer)
                for (i = 0; i < ETH_PACKETS_BATCH_RECV; i++)
--
2.37.2