[PATCH v2] lsefi: fixed memory leaks

[PATCH v2] lsefi: fixed memory leaks

[Renaud Métrich via Grub-devel]

Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
---
 grub-core/commands/efi/lsefi.c |  4 ++++
 include/grub/efi/efi.h         | 27 +++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/grub-core/commands/efi/lsefi.c b/grub-core/commands/efi/lsefi.c
index 7b8316d41..bda25a3a9 100644
--- a/grub-core/commands/efi/lsefi.c
+++ b/grub-core/commands/efi/lsefi.c
@@ -127,8 +127,12 @@ grub_cmd_lsefi (grub_command_t cmd __attribute__ ((unused)),
 	    grub_printf ("  %pG\n", protocols[j]);
 	}
 
+      if (protocols)
+	grub_efi_free_pool (protocols);
     }
 
+  grub_free (handles);
+
   return 0;
 }
 
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index b4d2f2d46..538bdcda8 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -71,6 +71,7 @@ EXPORT_FUNC(grub_efi_get_memory_map) (grub_efi_uintn_t *memory_map_size,
 				      grub_efi_uintn_t *descriptor_size,
 				      grub_efi_uint32_t *descriptor_version);
 void grub_efi_memory_fini (void);
+
 grub_efi_loaded_image_t *EXPORT_FUNC(grub_efi_get_loaded_image) (grub_efi_handle_t image_handle);
 void EXPORT_FUNC(grub_efi_print_device_path) (grub_efi_device_path_t *dp);
 char *EXPORT_FUNC(grub_efi_get_filename) (grub_efi_device_path_t *dp);
@@ -193,4 +194,30 @@ struct grub_net_card;
 grub_efi_handle_t
 grub_efinet_get_device_handle (struct grub_net_card *card);
 
+static inline grub_efi_status_t
+__attribute__((__unused__))
+grub_efi_allocate_pool (grub_efi_memory_type_t pool_type,
+			grub_efi_uintn_t buffer_size,
+			void **buffer)
+{
+  grub_efi_boot_services_t *b;
+  grub_efi_status_t status;
+
+  b = grub_efi_system_table->boot_services;
+  status = b->allocate_pool(pool_type, buffer_size, buffer);
+  return status;
+}
+
+static inline grub_efi_status_t
+__attribute__((__unused__))
+grub_efi_free_pool (void *buffer)
+{
+  grub_efi_boot_services_t *b;
+  grub_efi_status_t status;
+
+  b = grub_efi_system_table->boot_services;
+  status = b->free_pool(buffer);
+  return status;
+}
+
 #endif /* ! GRUB_EFI_EFI_HEADER */
-- 
2.51.0


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] lsefi: fixed memory leaks

[Renaud Métrich via Grub-devel]

[-- Attachment #1.1.1: Type: text/plain, Size: 2522 bytes --]

Hello,

I'm very sorry for having submitted a broken patch earlier, I didn't 
notice I had not committed the efi.h file which is now defining the 
required grub_efi_free_pool() function.

Renaud.

Le 9/2/25 à 1:55 PM, Renaud Métrich a écrit :
> Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
> ---
>   grub-core/commands/efi/lsefi.c |  4 ++++
>   include/grub/efi/efi.h         | 27 +++++++++++++++++++++++++++
>   2 files changed, 31 insertions(+)
>
> diff --git a/grub-core/commands/efi/lsefi.c b/grub-core/commands/efi/lsefi.c
> index 7b8316d41..bda25a3a9 100644
> --- a/grub-core/commands/efi/lsefi.c
> +++ b/grub-core/commands/efi/lsefi.c
> @@ -127,8 +127,12 @@ grub_cmd_lsefi (grub_command_t cmd __attribute__ ((unused)),
>   	    grub_printf ("  %pG\n", protocols[j]);
>   	}
>   
> +      if (protocols)
> +	grub_efi_free_pool (protocols);
>       }
>   
> +  grub_free (handles);
> +
>     return 0;
>   }
>   
> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> index b4d2f2d46..538bdcda8 100644
> --- a/include/grub/efi/efi.h
> +++ b/include/grub/efi/efi.h
> @@ -71,6 +71,7 @@ EXPORT_FUNC(grub_efi_get_memory_map) (grub_efi_uintn_t *memory_map_size,
>   				      grub_efi_uintn_t *descriptor_size,
>   				      grub_efi_uint32_t *descriptor_version);
>   void grub_efi_memory_fini (void);
> +
>   grub_efi_loaded_image_t *EXPORT_FUNC(grub_efi_get_loaded_image) (grub_efi_handle_t image_handle);
>   void EXPORT_FUNC(grub_efi_print_device_path) (grub_efi_device_path_t *dp);
>   char *EXPORT_FUNC(grub_efi_get_filename) (grub_efi_device_path_t *dp);
> @@ -193,4 +194,30 @@ struct grub_net_card;
>   grub_efi_handle_t
>   grub_efinet_get_device_handle (struct grub_net_card *card);
>   
> +static inline grub_efi_status_t
> +__attribute__((__unused__))
> +grub_efi_allocate_pool (grub_efi_memory_type_t pool_type,
> +			grub_efi_uintn_t buffer_size,
> +			void **buffer)
> +{
> +  grub_efi_boot_services_t *b;
> +  grub_efi_status_t status;
> +
> +  b = grub_efi_system_table->boot_services;
> +  status = b->allocate_pool(pool_type, buffer_size, buffer);
> +  return status;
> +}
> +
> +static inline grub_efi_status_t
> +__attribute__((__unused__))
> +grub_efi_free_pool (void *buffer)
> +{
> +  grub_efi_boot_services_t *b;
> +  grub_efi_status_t status;
> +
> +  b = grub_efi_system_table->boot_services;
> +  status = b->free_pool(buffer);
> +  return status;
> +}
> +
>   #endif /* ! GRUB_EFI_EFI_HEADER */

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] lsefi: fixed memory leaks

[Daniel Kiper]

On Tue, Sep 02, 2025 at 01:55:15PM +0200, Renaud Métrich via Grub-devel wrote:
> Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
> ---
>  grub-core/commands/efi/lsefi.c |  4 ++++
>  include/grub/efi/efi.h         | 27 +++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
>
> diff --git a/grub-core/commands/efi/lsefi.c b/grub-core/commands/efi/lsefi.c
> index 7b8316d41..bda25a3a9 100644
> --- a/grub-core/commands/efi/lsefi.c
> +++ b/grub-core/commands/efi/lsefi.c
> @@ -127,8 +127,12 @@ grub_cmd_lsefi (grub_command_t cmd __attribute__ ((unused)),
>  	    grub_printf ("  %pG\n", protocols[j]);
>  	}
>
> +      if (protocols)
> +	grub_efi_free_pool (protocols);
>      }
>
> +  grub_free (handles);
> +
>    return 0;
>  }
>
> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> index b4d2f2d46..538bdcda8 100644
> --- a/include/grub/efi/efi.h
> +++ b/include/grub/efi/efi.h
> @@ -71,6 +71,7 @@ EXPORT_FUNC(grub_efi_get_memory_map) (grub_efi_uintn_t *memory_map_size,
>  				      grub_efi_uintn_t *descriptor_size,
>  				      grub_efi_uint32_t *descriptor_version);
>  void grub_efi_memory_fini (void);
> +

Please drop this noise...

>  grub_efi_loaded_image_t *EXPORT_FUNC(grub_efi_get_loaded_image) (grub_efi_handle_t image_handle);
>  void EXPORT_FUNC(grub_efi_print_device_path) (grub_efi_device_path_t *dp);
>  char *EXPORT_FUNC(grub_efi_get_filename) (grub_efi_device_path_t *dp);
> @@ -193,4 +194,30 @@ struct grub_net_card;
>  grub_efi_handle_t
>  grub_efinet_get_device_handle (struct grub_net_card *card);
>
> +static inline grub_efi_status_t
> +__attribute__((__unused__))

Please drop this attribute. And if you introduce this function please do
this in separate patch and add its call in grub-core/kern/efi/mm.c.

> +grub_efi_allocate_pool (grub_efi_memory_type_t pool_type,
> +			grub_efi_uintn_t buffer_size,
> +			void **buffer)
> +{
> +  grub_efi_boot_services_t *b;
> +  grub_efi_status_t status;
> +
> +  b = grub_efi_system_table->boot_services;
> +  status = b->allocate_pool(pool_type, buffer_size, buffer);

Missing space before "(".

> +  return status;
> +}
> +
> +static inline grub_efi_status_t
> +__attribute__((__unused__))

This attribute is not needed.

> +grub_efi_free_pool (void *buffer)
> +{
> +  grub_efi_boot_services_t *b;
> +  grub_efi_status_t status;
> +
> +  b = grub_efi_system_table->boot_services;
> +  status = b->free_pool(buffer);

Missing space before "(".

> +  return status;
> +}
> +
>  #endif /* ! GRUB_EFI_EFI_HEADER */

FYI, I am planning code freeze on 3rd of October 2025. If you want me to
take your patches into the upcoming release I suggest to speed up a turn
over...

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] lsefi: fixed memory leaks

[Renaud Métrich via Grub-devel]

[-- Attachment #1.1.1: Type: text/plain, Size: 3218 bytes --]

Hi Daniel,

This code requires porting of functions available in RHEL fork only: 
grub_efi_free_pool() and grub_efi_allocate_pool()

Discussing with Marta Lewando from Red Hat, she told me this will be a 
separate patch, hence we shall defer until that patch is merged with 
Upstream.

Renaud.

Le 9/4/25 à 3:59 PM, Daniel Kiper a écrit :
> On Tue, Sep 02, 2025 at 01:55:15PM +0200, Renaud Métrich via Grub-devel wrote:
>> Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
>> ---
>>   grub-core/commands/efi/lsefi.c |  4 ++++
>>   include/grub/efi/efi.h         | 27 +++++++++++++++++++++++++++
>>   2 files changed, 31 insertions(+)
>>
>> diff --git a/grub-core/commands/efi/lsefi.c b/grub-core/commands/efi/lsefi.c
>> index 7b8316d41..bda25a3a9 100644
>> --- a/grub-core/commands/efi/lsefi.c
>> +++ b/grub-core/commands/efi/lsefi.c
>> @@ -127,8 +127,12 @@ grub_cmd_lsefi (grub_command_t cmd __attribute__ ((unused)),
>>   	    grub_printf ("  %pG\n", protocols[j]);
>>   	}
>>
>> +      if (protocols)
>> +	grub_efi_free_pool (protocols);
>>       }
>>
>> +  grub_free (handles);
>> +
>>     return 0;
>>   }
>>
>> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
>> index b4d2f2d46..538bdcda8 100644
>> --- a/include/grub/efi/efi.h
>> +++ b/include/grub/efi/efi.h
>> @@ -71,6 +71,7 @@ EXPORT_FUNC(grub_efi_get_memory_map) (grub_efi_uintn_t *memory_map_size,
>>   				      grub_efi_uintn_t *descriptor_size,
>>   				      grub_efi_uint32_t *descriptor_version);
>>   void grub_efi_memory_fini (void);
>> +
> Please drop this noise...
>
>>   grub_efi_loaded_image_t *EXPORT_FUNC(grub_efi_get_loaded_image) (grub_efi_handle_t image_handle);
>>   void EXPORT_FUNC(grub_efi_print_device_path) (grub_efi_device_path_t *dp);
>>   char *EXPORT_FUNC(grub_efi_get_filename) (grub_efi_device_path_t *dp);
>> @@ -193,4 +194,30 @@ struct grub_net_card;
>>   grub_efi_handle_t
>>   grub_efinet_get_device_handle (struct grub_net_card *card);
>>
>> +static inline grub_efi_status_t
>> +__attribute__((__unused__))
> Please drop this attribute. And if you introduce this function please do
> this in separate patch and add its call in grub-core/kern/efi/mm.c.
>
>> +grub_efi_allocate_pool (grub_efi_memory_type_t pool_type,
>> +			grub_efi_uintn_t buffer_size,
>> +			void **buffer)
>> +{
>> +  grub_efi_boot_services_t *b;
>> +  grub_efi_status_t status;
>> +
>> +  b = grub_efi_system_table->boot_services;
>> +  status = b->allocate_pool(pool_type, buffer_size, buffer);
> Missing space before "(".
>
>> +  return status;
>> +}
>> +
>> +static inline grub_efi_status_t
>> +__attribute__((__unused__))
> This attribute is not needed.
>
>> +grub_efi_free_pool (void *buffer)
>> +{
>> +  grub_efi_boot_services_t *b;
>> +  grub_efi_status_t status;
>> +
>> +  b = grub_efi_system_table->boot_services;
>> +  status = b->free_pool(buffer);
> Missing space before "(".
>
>> +  return status;
>> +}
>> +
>>   #endif /* ! GRUB_EFI_EFI_HEADER */
> FYI, I am planning code freeze on 3rd of October 2025. If you want me to
> take your patches into the upcoming release I suggest to speed up a turn
> over...
>
> Daniel
>

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel