[PATCH 1/1] efi_loader: fix handling of DHCP acknowledge
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++)
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
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
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
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
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
participants (2)
-
Heinrich Schuchardt -
Ilias Apalodimas