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

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

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
- shadowing of the global netobj 
Can we please update the commit message to something that mentions all of
these?

Thanks
/Ilias

> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@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
>