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

[Ilias Apalodimas]

On Wed, Nov 30, 2022 at 08:50:07AM +0100, Heinrich Schuchardt wrote:
> 
> 
> 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.
> 

Ah missed that

Thanks!
/Ilias
> 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
> > >