[PATCH v3] command/extcmd: perform explicit NULL check in both the unregister helpers

[PATCH v3] command/extcmd: perform explicit NULL check in both the unregister helpers

[Srish Srinivasan]

During command registration, grub_register_command_prio
returns a 0 when there is a failure in memory allocation.
In such a situation, calls to grub_unregister_{command(),
extcmd()} during command unregistration will result in
dereferencing a NULL pointer.

Perform explicit NULL check in both the unregister helpers to
prevent undefined behaviour due to a NULL pointer dereference.

Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
Reviewed-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
 grub-core/commands/extcmd.c | 3 +++
 grub-core/kern/command.c    | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/grub-core/commands/extcmd.c b/grub-core/commands/extcmd.c
index c236be13a..7f8cb3f67 100644
--- a/grub-core/commands/extcmd.c
+++ b/grub-core/commands/extcmd.c
@@ -139,6 +139,9 @@ grub_register_extcmd_lockdown (const char *name, grub_extcmd_func_t func,
 void
 grub_unregister_extcmd (grub_extcmd_t ext)
 {
+  if (ext == NULL)
+    return;
+
   grub_unregister_command (ext->cmd);
   grub_free (ext);
 }
diff --git a/grub-core/kern/command.c b/grub-core/kern/command.c
index 5812e131c..a1b4c81a9 100644
--- a/grub-core/kern/command.c
+++ b/grub-core/kern/command.c
@@ -104,6 +104,9 @@ grub_register_command_lockdown (const char *name,
 void
 grub_unregister_command (grub_command_t cmd)
 {
+  if (cmd == NULL)
+    return;
+
   if ((cmd->prio & GRUB_COMMAND_FLAG_ACTIVE) && (cmd->next))
     cmd->next->prio |= GRUB_COMMAND_FLAG_ACTIVE;
   grub_list_remove (GRUB_AS_LIST (cmd));
-- 
2.51.0

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

Re: [PATCH v3] command/extcmd: perform explicit NULL check in both the unregister helpers

[Andrew Hamilton]

Looks good to me, thank you.

Reviewed-by: Andrew Hamilton <adhamilt@gmail.com>

On Mon, Sep 22, 2025 at 1:04 AM Srish Srinivasan <ssrish@linux.ibm.com> wrote:
>
> During command registration, grub_register_command_prio
> returns a 0 when there is a failure in memory allocation.
> In such a situation, calls to grub_unregister_{command(),
> extcmd()} during command unregistration will result in
> dereferencing a NULL pointer.
>
> Perform explicit NULL check in both the unregister helpers to
> prevent undefined behaviour due to a NULL pointer dereference.
>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  grub-core/commands/extcmd.c | 3 +++
>  grub-core/kern/command.c    | 3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/grub-core/commands/extcmd.c b/grub-core/commands/extcmd.c
> index c236be13a..7f8cb3f67 100644
> --- a/grub-core/commands/extcmd.c
> +++ b/grub-core/commands/extcmd.c
> @@ -139,6 +139,9 @@ grub_register_extcmd_lockdown (const char *name, grub_extcmd_func_t func,
>  void
>  grub_unregister_extcmd (grub_extcmd_t ext)
>  {
> +  if (ext == NULL)
> +    return;
> +
>    grub_unregister_command (ext->cmd);
>    grub_free (ext);
>  }
> diff --git a/grub-core/kern/command.c b/grub-core/kern/command.c
> index 5812e131c..a1b4c81a9 100644
> --- a/grub-core/kern/command.c
> +++ b/grub-core/kern/command.c
> @@ -104,6 +104,9 @@ grub_register_command_lockdown (const char *name,
>  void
>  grub_unregister_command (grub_command_t cmd)
>  {
> +  if (cmd == NULL)
> +    return;
> +
>    if ((cmd->prio & GRUB_COMMAND_FLAG_ACTIVE) && (cmd->next))
>      cmd->next->prio |= GRUB_COMMAND_FLAG_ACTIVE;
>    grub_list_remove (GRUB_AS_LIST (cmd));
> --
> 2.51.0
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

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

Re: [PATCH v3] command/extcmd: perform explicit NULL check in both the unregister helpers

[Daniel Kiper]

On Mon, Sep 22, 2025 at 11:32:46AM +0530, Srish Srinivasan wrote:
> During command registration, grub_register_command_prio
> returns a 0 when there is a failure in memory allocation.
> In such a situation, calls to grub_unregister_{command(),
> extcmd()} during command unregistration will result in
> dereferencing a NULL pointer.
>
> Perform explicit NULL check in both the unregister helpers to
> prevent undefined behaviour due to a NULL pointer dereference.
>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

Patch LGTM to me but it does not seem to be posted earlier to the
grub-devel. So, I cannot accept Sudhakar's and Stefan's RBs until they
are confirmed here.

Though Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...

Daniel

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

Re: [PATCH v3] command/extcmd: perform explicit NULL check in both the unregister helpers

[Srish Srinivasan]

On 9/29/25 5:25 PM, Daniel Kiper wrote:
> On Mon, Sep 22, 2025 at 11:32:46AM +0530, Srish Srinivasan wrote:
>> During command registration, grub_register_command_prio
>> returns a 0 when there is a failure in memory allocation.
>> In such a situation, calls to grub_unregister_{command(),
>> extcmd()} during command unregistration will result in
>> dereferencing a NULL pointer.
>>
>> Perform explicit NULL check in both the unregister helpers to
>> prevent undefined behaviour due to a NULL pointer dereference.
>>
>> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
>> Reviewed-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Patch LGTM to me but it does not seem to be posted earlier to the
> grub-devel. So, I cannot accept Sudhakar's and Stefan's RBs until they
> are confirmed here.
>
> Though Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...
>
> Daniel
Hi Daniel,
thank you for the reviewed-by.

And, here are the links to v2 for the same patch where I got reviews 
from Sudhakar and Stefan.

  Sudhakar's reviewed-by: 
https://lists.gnu.org/archive/html/grub-devel/2025-09/msg00087.html
  Stefan's reviewed-by: 
https://lists.gnu.org/archive/html/grub-devel/2025-09/msg00113.html

And, here is the link to v1 for the same patch just for your reference.

  https://lists.gnu.org/archive/html/grub-devel/2025-09/msg00072.html

Hope this helps.
Thank You.
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

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

Re: [PATCH v3] command/extcmd: perform explicit NULL check in both the unregister helpers

[Daniel Kiper]

On Mon, Sep 29, 2025 at 05:47:59PM +0530, Srish Srinivasan wrote:
> On 9/29/25 5:25 PM, Daniel Kiper wrote:
> > On Mon, Sep 22, 2025 at 11:32:46AM +0530, Srish Srinivasan wrote:
> > > During command registration, grub_register_command_prio
> > > returns a 0 when there is a failure in memory allocation.
> > > In such a situation, calls to grub_unregister_{command(),
> > > extcmd()} during command unregistration will result in
> > > dereferencing a NULL pointer.
> > >
> > > Perform explicit NULL check in both the unregister helpers to
> > > prevent undefined behaviour due to a NULL pointer dereference.
> > >
> > > Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> > > Reviewed-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
> > > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> > Patch LGTM to me but it does not seem to be posted earlier to the
> > grub-devel. So, I cannot accept Sudhakar's and Stefan's RBs until they
> > are confirmed here.
> >
> > Though Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...
> >
> > Daniel
> Hi Daniel,
> thank you for the reviewed-by.
>
> And, here are the links to v2 for the same patch where I got reviews from
> Sudhakar and Stefan.
>
>  Sudhakar's reviewed-by:
> https://lists.gnu.org/archive/html/grub-devel/2025-09/msg00087.html
>  Stefan's reviewed-by:
> https://lists.gnu.org/archive/html/grub-devel/2025-09/msg00113.html
>
> And, here is the link to v1 for the same patch just for your reference.
>
>  https://lists.gnu.org/archive/html/grub-devel/2025-09/msg00072.html
>
> Hope this helps.

Sorry, I missed both emails due to subject change. I will get Sudhakar's
and Stefan's RBs then.

Daniel

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