* [PATCH 1/1] efi_loader: fix handling of DHCP acknowledge
@ 2022-11-26 16:10 Heinrich Schuchardt
2022-11-30 7:37 ` Ilias Apalodimas
0 siblings, 1 reply; 6+ messages in thread
From: Heinrich Schuchardt @ 2022-11-26 16:10 UTC (permalink / raw)
To: Ilias Apalodimas; +Cc: u-boot, Heinrich Schuchardt
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.
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] efi_loader: fix handling of DHCP acknowledge
2022-11-26 16:10 [PATCH 1/1] efi_loader: fix handling of DHCP acknowledge Heinrich Schuchardt
@ 2022-11-30 7:37 ` Ilias Apalodimas
2022-11-30 7:50 ` Heinrich Schuchardt
0 siblings, 1 reply; 6+ messages in thread
From: Ilias Apalodimas @ 2022-11-30 7:37 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: u-boot
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] efi_loader: fix handling of DHCP acknowledge
2022-11-30 7:37 ` Ilias Apalodimas
@ 2022-11-30 7:50 ` Heinrich Schuchardt
2022-11-30 8:44 ` Ilias Apalodimas
0 siblings, 1 reply; 6+ messages in thread
From: Heinrich Schuchardt @ 2022-11-30 7:50 UTC (permalink / raw)
To: Ilias Apalodimas; +Cc: u-boot
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@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
>>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] efi_loader: fix handling of DHCP acknowledge
2022-11-30 7:50 ` Heinrich Schuchardt
@ 2022-11-30 8:44 ` Ilias Apalodimas
2022-11-30 12:45 ` Heinrich Schuchardt
0 siblings, 1 reply; 6+ messages in thread
From: Ilias Apalodimas @ 2022-11-30 8:44 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: u-boot
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@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
> > >
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] efi_loader: fix handling of DHCP acknowledge
2022-11-30 8:44 ` Ilias Apalodimas
@ 2022-11-30 12:45 ` Heinrich Schuchardt
2022-12-01 7:43 ` Ilias Apalodimas
0 siblings, 1 reply; 6+ messages in thread
From: Heinrich Schuchardt @ 2022-11-30 12:45 UTC (permalink / raw)
To: Ilias Apalodimas; +Cc: u-boot
On 11/30/22 09:44, Ilias Apalodimas wrote:
> 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
Above I wrote "Handle out of memory."
Regards
Heinrich
>>
>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] efi_loader: fix handling of DHCP acknowledge
2022-11-30 12:45 ` Heinrich Schuchardt
@ 2022-12-01 7:43 ` Ilias Apalodimas
0 siblings, 0 replies; 6+ messages in thread
From: Ilias Apalodimas @ 2022-12-01 7:43 UTC (permalink / raw)
To: Heinrich Schuchardt; +Cc: u-boot
Hi Henrich
On Wed, 30 Nov 2022 at 14:45, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> On 11/30/22 09:44, Ilias Apalodimas wrote:
> > 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
>
> Above I wrote "Handle out of memory."
ah, well that wasn't too clear when I first read the commit message.
Can you please amend ti while merging?
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> Regards
>
> Heinrich
>
>
> >>
> >> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-01 7:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-26 16:10 [PATCH 1/1] efi_loader: fix handling of DHCP acknowledge Heinrich Schuchardt
2022-11-30 7:37 ` Ilias Apalodimas
2022-11-30 7:50 ` Heinrich Schuchardt
2022-11-30 8:44 ` Ilias Apalodimas
2022-11-30 12:45 ` Heinrich Schuchardt
2022-12-01 7:43 ` Ilias Apalodimas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.