Re: [PATCH] normal/cmdline: fix hist_lines state loss and grub calloc failure check

> On 20 Nov 2025, at 8:42 PM, Avnish Chouhan <[email protected]> wrote:
> 
> On 2025-11-20 15:38, Sudhakar Kuppusamy wrote:
>>> On 20 Nov 2025, at 1:41 PM, Avnish Chouhan <[email protected]> wrote:
>>> Adding a fix for hist_lines memory leak and state loss. In
>>> current code, we overwrite hist_lines before checking the
>>> allocation result. If grub_calloc fails, hist_lines becomes NULL
>>> and we loose the reference to the previously allocated hist_lines.
>>> With this new change. On failure, hist_lines remains pointing to
>>> the old valid memory. No leak, no state corruption.
>>> Along with this, adding a failure check in grub_calloc(). If
>>> grub_calloc fails, (e.g., due to memory allocation failure),
>>> it returns NULL. Then, passing hist_lines (which would be NULL)
>>> to grub_memmove() will result in a null pointer dereference,
>>> and can cause an undefined behavior.
>> Sorry, Avnish. I don’t understand the above commit message. Could you
>> please rephrase it.
>> Thanks,
>> Sudhakar
> 
> This commit message clearly defines the problem and the intended purpose of this patch!

In the first message you defines the problem and also mentioned the solution.
The second message is confusing to me.

> 
>>> Signed-off-by: Avnish Chouhan <[email protected]>
>>> ---
>>> grub-core/normal/cmdline.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>> diff --git a/grub-core/normal/cmdline.c b/grub-core/normal/cmdline.c
>>> index 9c6d9ad..14a40a4 100644
>>> --- a/grub-core/normal/cmdline.c
>>> +++ b/grub-core/normal/cmdline.c
>>> @@ -42,7 +42,14 @@ grub_err_t
>>> grub_set_history (int newsize)
>>> {
>>>  grub_uint32_t **old_hist_lines = hist_lines;
>>> +
>>>  hist_lines = grub_calloc (newsize, sizeof (grub_uint32_t *));
>>> +  if (hist_lines == NULL)
>>> +    {
>>> +      /* We need to restore hist_lines to avoid memory leak and state loss */
>>> +      hist_lines = old_hist_lines;
>>> +      return grub_errno;
>>> +    }
>>>  /* Copy the old lines into the new buffer.  */
>>>  if (old_hist_lines)
>>> --
>>> 2.47.1
>>> _______________________________________________
>>> Grub-devel mailing list
>>> [email protected]
>>> https://lists.gnu.org/mailman/listinfo/grub-devel



_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel