[PATCH 1/1] efi_loader: efi_dp_part_node check dp_alloc return value
dp_alloc() may return NULL. This needs to be caught.
Fixes: 98d48bdf415e ("efi_loader: provide a function to create a partition node") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- lib/efi_loader/efi_device_path.c | 3 ++- lib/efi_loader/efi_disk.c | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index ebffb77122..acae007f26 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -936,7 +936,8 @@ struct efi_device_path *efi_dp_part_node(struct blk_desc *desc, int part) dpsize = sizeof(struct efi_device_path_hard_drive_path); buf = dp_alloc(dpsize);
- dp_part_node(buf, desc, part); + if (buf) + dp_part_node(buf, desc, part);
return buf; } diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 39ea1a68a6..79b28097b6 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -415,6 +415,11 @@ static efi_status_t efi_disk_add_dev( struct efi_handler *handler; void *protocol_interface;
+ if (!node) { + ret = EFI_OUT_OF_RESOURCES; + goto error; + } + /* Parent must expose EFI_BLOCK_IO_PROTOCOL */ ret = efi_search_protocol(parent, &efi_block_io_guid, &handler); if (ret != EFI_SUCCESS)
On Thu, 6 Oct 2022 at 14:45, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
dp_alloc() may return NULL. This needs to be caught.
Fixes: 98d48bdf415e ("efi_loader: provide a function to create a partition node") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_device_path.c | 3 ++- lib/efi_loader/efi_disk.c | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index ebffb77122..acae007f26 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -936,7 +936,8 @@ struct efi_device_path *efi_dp_part_node(struct blk_desc *desc, int part) dpsize = sizeof(struct efi_device_path_hard_drive_path); buf = dp_alloc(dpsize);
dp_part_node(buf, desc, part);
if (buf)dp_part_node(buf, desc, part); return buf;} diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 39ea1a68a6..79b28097b6 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -415,6 +415,11 @@ static efi_status_t efi_disk_add_dev( struct efi_handler *handler; void *protocol_interface;
if (!node) {ret = EFI_OUT_OF_RESOURCES;goto error;}
There's a diskobj that has been allocated here and a handle we added. We should clean those up too
Cheers /Ilias
/* Parent must expose EFI_BLOCK_IO_PROTOCOL */ ret = efi_search_protocol(parent, &efi_block_io_guid, &handler); if (ret != EFI_SUCCESS)-- 2.37.2
On 10/6/22 13:59, Ilias Apalodimas wrote:
On Thu, 6 Oct 2022 at 14:45, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
dp_alloc() may return NULL. This needs to be caught.
Fixes: 98d48bdf415e ("efi_loader: provide a function to create a partition node") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_device_path.c | 3 ++- lib/efi_loader/efi_disk.c | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index ebffb77122..acae007f26 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -936,7 +936,8 @@ struct efi_device_path *efi_dp_part_node(struct blk_desc *desc, int part) dpsize = sizeof(struct efi_device_path_hard_drive_path); buf = dp_alloc(dpsize);
dp_part_node(buf, desc, part);
if (buf)dp_part_node(buf, desc, part); return buf;}
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 39ea1a68a6..79b28097b6 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -415,6 +415,11 @@ static efi_status_t efi_disk_add_dev( struct efi_handler *handler; void *protocol_interface;
if (!node) {ret = EFI_OUT_OF_RESOURCES;goto error;}There's a diskobj that has been allocated here and a handle we added. We should clean those up too
512 error: 513 efi_delete_handle(&diskobj->header);
What is missing here?
Best regards
Heinrich
On Thu, 6 Oct 2022 at 15:18, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 10/6/22 13:59, Ilias Apalodimas wrote:
On Thu, 6 Oct 2022 at 14:45, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
dp_alloc() may return NULL. This needs to be caught.
Fixes: 98d48bdf415e ("efi_loader: provide a function to create a partition node") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_device_path.c | 3 ++- lib/efi_loader/efi_disk.c | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index ebffb77122..acae007f26 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -936,7 +936,8 @@ struct efi_device_path *efi_dp_part_node(struct blk_desc *desc, int part) dpsize = sizeof(struct efi_device_path_hard_drive_path); buf = dp_alloc(dpsize);
dp_part_node(buf, desc, part);
if (buf)dp_part_node(buf, desc, part); return buf;}
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 39ea1a68a6..79b28097b6 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -415,6 +415,11 @@ static efi_status_t efi_disk_add_dev( struct efi_handler *handler; void *protocol_interface;
if (!node) {ret = EFI_OUT_OF_RESOURCES;goto error;}There's a diskobj that has been allocated here and a handle we added. We should clean those up too
512 error: 513 efi_delete_handle(&diskobj->header);
What is missing here?
If deleting the handle fails we won't free that memory
Regards /Ilias
Best regards
Heinrich
On 10/6/22 14:49, Ilias Apalodimas wrote:
On Thu, 6 Oct 2022 at 15:18, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 10/6/22 13:59, Ilias Apalodimas wrote:
On Thu, 6 Oct 2022 at 14:45, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
dp_alloc() may return NULL. This needs to be caught.
Fixes: 98d48bdf415e ("efi_loader: provide a function to create a partition node") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_device_path.c | 3 ++- lib/efi_loader/efi_disk.c | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index ebffb77122..acae007f26 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -936,7 +936,8 @@ struct efi_device_path *efi_dp_part_node(struct blk_desc *desc, int part) dpsize = sizeof(struct efi_device_path_hard_drive_path); buf = dp_alloc(dpsize);
dp_part_node(buf, desc, part);
if (buf)dp_part_node(buf, desc, part); return buf;}
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 39ea1a68a6..79b28097b6 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -415,6 +415,11 @@ static efi_status_t efi_disk_add_dev( struct efi_handler *handler; void *protocol_interface;
if (!node) {ret = EFI_OUT_OF_RESOURCES;goto error;}There's a diskobj that has been allocated here and a handle we added. We should clean those up too
512 error: 513 efi_delete_handle(&diskobj->header);
What is missing here?
If deleting the handle fails we won't free that memory
If deleting succeeds, we won't free it either. And we don't reclaim the memory on the success path.
Not reclaiming the device-path is a separate bug to what I am looking at here.
Best regards
Heinrich
participants (2)
-
Heinrich Schuchardt -
Ilias Apalodimas