[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.schuchardt at 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
>>