* Re: [PATCH] blsuki: Fix grub_errno leakage in blsuki_is_default_entry()
2025-11-10 7:38 Michael Chang via Grub-devel
@ 2025-11-10 13:35 ` Sudhakar Kuppusamy
2025-11-11 4:38 ` Michael Chang via Grub-devel
2025-11-10 14:55 ` Michael Lawnick via Grub-devel
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Sudhakar Kuppusamy @ 2025-11-10 13:35 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Michael Chang
> On 10 Nov 2025, at 1:08 PM, Michael Chang via Grub-devel <grub-devel@gnu.org> wrote:
>
> The grub_strtol() call in blsuki_is_default_entry() can set grub_errno
> to GRUB_ERR_BAD_NUMBER if the input string cannot be converted into any
> valid digits.
>
> This errno value is currently left uncleared, which can lead to
> unexpected behavior in subsequent logic that tests the result of a
> function by checking grub_errno.
>
> Clear grub_errno and return false when GRUB_ERR_BAD_NUMBER is set, as
> this specific error should be ignored in this context.
>
> Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
Thanks,
Sudhakar
> ---
> grub-core/commands/blsuki.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/grub-core/commands/blsuki.c b/grub-core/commands/blsuki.c
> index 21d767f05..df25b6fbc 100644
> --- a/grub-core/commands/blsuki.c
> +++ b/grub-core/commands/blsuki.c
> @@ -1510,6 +1510,12 @@ blsuki_is_default_entry (const char *def_entry, grub_blsuki_entry_t *entry, int
> return true;
>
> def_idx = grub_strtol (def_entry, &def_entry_end, 0);
> + if (grub_errno == GRUB_ERR_BAD_NUMBER)
> + {
> + grub_errno = GRUB_ERR_NONE;
> + return false;
> + }
> +
> if (*def_entry_end != '\0' || def_idx < 0 || def_idx > GRUB_INT_MAX)
> return false;
>
> --
> 2.51.1
>
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] blsuki: Fix grub_errno leakage in blsuki_is_default_entry()
2025-11-10 13:35 ` Sudhakar Kuppusamy
@ 2025-11-11 4:38 ` Michael Chang via Grub-devel
0 siblings, 0 replies; 10+ messages in thread
From: Michael Chang via Grub-devel @ 2025-11-11 4:38 UTC (permalink / raw)
To: Sudhakar Kuppusamy; +Cc: Michael Chang, The development of GNU GRUB
On Mon, Nov 10, 2025 at 07:05:10PM +0530, Sudhakar Kuppusamy wrote:
>
>
> > On 10 Nov 2025, at 1:08 PM, Michael Chang via Grub-devel <grub-devel@gnu.org> wrote:
> >
> > The grub_strtol() call in blsuki_is_default_entry() can set grub_errno
> > to GRUB_ERR_BAD_NUMBER if the input string cannot be converted into any
> > valid digits.
> >
> > This errno value is currently left uncleared, which can lead to
> > unexpected behavior in subsequent logic that tests the result of a
> > function by checking grub_errno.
> >
> > Clear grub_errno and return false when GRUB_ERR_BAD_NUMBER is set, as
> > this specific error should be ignored in this context.
> >
> > Signed-off-by: Michael Chang <mchang@suse.com>
>
> Reviewed-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
Thanks for the review. I'll add your Reviewed-by tag in next version.
Thanks,
Michael
>
>
> Thanks,
> Sudhakar
> > ---
> > grub-core/commands/blsuki.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/grub-core/commands/blsuki.c b/grub-core/commands/blsuki.c
> > index 21d767f05..df25b6fbc 100644
> > --- a/grub-core/commands/blsuki.c
> > +++ b/grub-core/commands/blsuki.c
> > @@ -1510,6 +1510,12 @@ blsuki_is_default_entry (const char *def_entry, grub_blsuki_entry_t *entry, int
> > return true;
> >
> > def_idx = grub_strtol (def_entry, &def_entry_end, 0);
> > + if (grub_errno == GRUB_ERR_BAD_NUMBER)
> > + {
> > + grub_errno = GRUB_ERR_NONE;
> > + return false;
> > + }
> > +
> > if (*def_entry_end != '\0' || def_idx < 0 || def_idx > GRUB_INT_MAX)
> > return false;
> >
> > --
> > 2.51.1
> >
> >
> > _______________________________________________
> > 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] blsuki: Fix grub_errno leakage in blsuki_is_default_entry()
2025-11-10 7:38 Michael Chang via Grub-devel
2025-11-10 13:35 ` Sudhakar Kuppusamy
@ 2025-11-10 14:55 ` Michael Lawnick via Grub-devel
2025-11-11 4:18 ` Michael Chang via Grub-devel
2025-11-10 17:26 ` Alec Brown via Grub-devel
2025-11-11 4:21 ` Michael Chang via Grub-devel
3 siblings, 1 reply; 10+ messages in thread
From: Michael Lawnick via Grub-devel @ 2025-11-10 14:55 UTC (permalink / raw)
To: grub-devel; +Cc: Michael Lawnick
Am 10.11.2025 um 08:38 schrieb Michael Chang via Grub-devel:
> The grub_strtol() call in blsuki_is_default_entry() can set grub_errno
> to GRUB_ERR_BAD_NUMBER if the input string cannot be converted into any
> valid digits.
>
> This errno value is currently left uncleared, which can lead to
> unexpected behavior in subsequent logic that tests the result of a
> function by checking grub_errno.
>
> Clear grub_errno and return false when GRUB_ERR_BAD_NUMBER is set, as
> this specific error should be ignored in this context.
>
> Signed-off-by: Michael Chang <mchang@suse.com>
> ---
> grub-core/commands/blsuki.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/grub-core/commands/blsuki.c b/grub-core/commands/blsuki.c
> index 21d767f05..df25b6fbc 100644
> --- a/grub-core/commands/blsuki.c
> +++ b/grub-core/commands/blsuki.c
> @@ -1510,6 +1510,12 @@ blsuki_is_default_entry (const char *def_entry, grub_blsuki_entry_t *entry, int
> return true;
>
> def_idx = grub_strtol (def_entry, &def_entry_end, 0);
> + if (grub_errno == GRUB_ERR_BAD_NUMBER)
> + {
> + grub_errno = GRUB_ERR_NONE;
> + return false;
> + }
> +
> if (*def_entry_end != '\0' || def_idx < 0 || def_idx > GRUB_INT_MAX)
> return false;
>
This way you'll clear a possible previous GRUB_ERR_BAD_NUMBER
Shouldn't it be this way:
return true;
+ old_err = grub_errno;
+ grub_errno = GRUB_ERR_NONE;
def_idx = grub_strtol (def_entry, &def_entry_end, 0);
+ if (grub_errno == GRUB_ERR_BAD_NUMBER)
+ {
+ grub_errno = old_err;
+ return false;
+ }
+ grub_errno = old_err;
+
if (*def_entry_end != '\0' || def_idx < 0 || def_idx > GRUB_INT_MAX)
return false;
JMTC
KR Michael
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] blsuki: Fix grub_errno leakage in blsuki_is_default_entry()
2025-11-10 14:55 ` Michael Lawnick via Grub-devel
@ 2025-11-11 4:18 ` Michael Chang via Grub-devel
0 siblings, 0 replies; 10+ messages in thread
From: Michael Chang via Grub-devel @ 2025-11-11 4:18 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Michael Chang, Michael Lawnick
On Mon, Nov 10, 2025 at 03:55:03PM +0100, Michael Lawnick via Grub-devel wrote:
>
>
> Am 10.11.2025 um 08:38 schrieb Michael Chang via Grub-devel:
> > The grub_strtol() call in blsuki_is_default_entry() can set grub_errno
> > to GRUB_ERR_BAD_NUMBER if the input string cannot be converted into any
> > valid digits.
> >
> > This errno value is currently left uncleared, which can lead to
> > unexpected behavior in subsequent logic that tests the result of a
> > function by checking grub_errno.
> >
> > Clear grub_errno and return false when GRUB_ERR_BAD_NUMBER is set, as
> > this specific error should be ignored in this context.
> >
> > Signed-off-by: Michael Chang <mchang@suse.com>
> > ---
> > grub-core/commands/blsuki.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/grub-core/commands/blsuki.c b/grub-core/commands/blsuki.c
> > index 21d767f05..df25b6fbc 100644
> > --- a/grub-core/commands/blsuki.c
> > +++ b/grub-core/commands/blsuki.c
> > @@ -1510,6 +1510,12 @@ blsuki_is_default_entry (const char *def_entry, grub_blsuki_entry_t *entry, int
> > return true;
> >
> > def_idx = grub_strtol (def_entry, &def_entry_end, 0);
> > + if (grub_errno == GRUB_ERR_BAD_NUMBER)
> > + {
> > + grub_errno = GRUB_ERR_NONE;
> > + return false;
> > + }
> > +
> > if (*def_entry_end != '\0' || def_idx < 0 || def_idx > GRUB_INT_MAX)
> > return false;
> >
> This way you'll clear a possible previous GRUB_ERR_BAD_NUMBER
> Shouldn't it be this way:
No, I think this patch does not introduce such behavior change. The
previous error of any kind is not considered an issue, and there was no
intent to preserve them unless grub_strtol() was explicitly surrounded
by grub_error_push()/grub_error_pop(), but apparantly it is not there
before the patch is applied. The error return is always overridden by
the error return code of grub_strtol() if there's any.
> return true;
>
> + old_err = grub_errno;
> + grub_errno = GRUB_ERR_NONE;
> def_idx = grub_strtol (def_entry, &def_entry_end, 0);
> + if (grub_errno == GRUB_ERR_BAD_NUMBER)
> + {
> + grub_errno = old_err;
> + return false;
> + }
> + grub_errno = old_err;
> +
IMHO restoring unhandled grub_errno here would still be considered a
bug. It could spread to other unrelated modules or functions which might
interpret it as an unspecified/unknown error and thus disrupt the boot
process in one way or another. This is exactly what the patch trying to
avoid.
Thanks,
Michael
> if (*def_entry_end != '\0' || def_idx < 0 || def_idx > GRUB_INT_MAX)
> return false;
>
> JMTC
> KR Michael
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] blsuki: Fix grub_errno leakage in blsuki_is_default_entry()
2025-11-10 7:38 Michael Chang via Grub-devel
2025-11-10 13:35 ` Sudhakar Kuppusamy
2025-11-10 14:55 ` Michael Lawnick via Grub-devel
@ 2025-11-10 17:26 ` Alec Brown via Grub-devel
2025-11-11 4:36 ` Michael Chang via Grub-devel
2025-11-11 4:21 ` Michael Chang via Grub-devel
3 siblings, 1 reply; 10+ messages in thread
From: Alec Brown via Grub-devel @ 2025-11-10 17:26 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Alec Brown, Michael Chang
On Mon, Nov 10, 2025 at 2:38 AM, Michael Chang <mchang@suse.com> wrote:
> The grub_strtol() call in blsuki_is_default_entry() can set grub_errno
> to GRUB_ERR_BAD_NUMBER if the input string cannot be converted into any
> valid digits.
>
> This errno value is currently left uncleared, which can lead to
> unexpected behavior in subsequent logic that tests the result of a
> function by checking grub_errno.
>
> Clear grub_errno and return false when GRUB_ERR_BAD_NUMBER is set, as
> this specific error should be ignored in this context.
grub_strtol() can also set grub_errno to GRUB_ERR_OUT_OF_RANGE. Would it make
sense to check for that as well? We do return false if def_idx is negative
or larger than GRUB_INT_MAX, but I don't know if we would want to sustain
GRUB_ERR_OUT_OF_RANGE in grub_errno past this function should the "def_entry"
passed into this function contain a bad value that causes grub_strtol() to give
an error.
Alec Brown
>
> Signed-off-by: Michael Chang <mchang@suse.com>
> ---
> grub-core/commands/blsuki.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/grub-core/commands/blsuki.c b/grub-core/commands/blsuki.c
> index 21d767f05..df25b6fbc 100644
> --- a/grub-core/commands/blsuki.c
> +++ b/grub-core/commands/blsuki.c
> @@ -1510,6 +1510,12 @@ blsuki_is_default_entry (const char *def_entry, grub_blsuki_entry_t *entry,
> int
> return true;
>
> def_idx = grub_strtol (def_entry, &def_entry_end, 0);
> + if (grub_errno == GRUB_ERR_BAD_NUMBER)
> + {
> + grub_errno = GRUB_ERR_NONE;
> + return false;
> + }
> +
> if (*def_entry_end != '\0' || def_idx < 0 || def_idx > GRUB_INT_MAX)
> return false;
>
> --
> 2.51.1
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://urldefense.com/v3/__https://lists.gnu.org/mailman/listinfo/grub-
> devel__;!!ACWV5N9M2RV99hQ!PT07VdaZpBxwaIforN0iXRL1OG2vC6xOV2SQ6cgn5eVQvHxGFpEtx3c9eNW_ApT4iml6Fnoj
> FNjVxarvCu1C$
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] blsuki: Fix grub_errno leakage in blsuki_is_default_entry()
2025-11-10 17:26 ` Alec Brown via Grub-devel
@ 2025-11-11 4:36 ` Michael Chang via Grub-devel
0 siblings, 0 replies; 10+ messages in thread
From: Michael Chang via Grub-devel @ 2025-11-11 4:36 UTC (permalink / raw)
To: Alec Brown; +Cc: Michael Chang, The development of GNU GRUB
On Mon, Nov 10, 2025 at 05:26:02PM +0000, Alec Brown wrote:
> On Mon, Nov 10, 2025 at 2:38 AM, Michael Chang <mchang@suse.com> wrote:
> > The grub_strtol() call in blsuki_is_default_entry() can set grub_errno
> > to GRUB_ERR_BAD_NUMBER if the input string cannot be converted into any
> > valid digits.
> >
> > This errno value is currently left uncleared, which can lead to
> > unexpected behavior in subsequent logic that tests the result of a
> > function by checking grub_errno.
> >
> > Clear grub_errno and return false when GRUB_ERR_BAD_NUMBER is set, as
> > this specific error should be ignored in this context.
>
> grub_strtol() can also set grub_errno to GRUB_ERR_OUT_OF_RANGE. Would it make
> sense to check for that as well? We do return false if def_idx is negative
> or larger than GRUB_INT_MAX, but I don't know if we would want to sustain
> GRUB_ERR_OUT_OF_RANGE in grub_errno past this function should the "def_entry"
> passed into this function contain a bad value that causes grub_strtol() to give
> an error.
Yes, you are right that GRUB_ERR_OUT_OF_RANGE could pass through the
function and cause the same issue as leaked grub_errno. I'll add a check
for this case in the next version.
Thanks,
Michael
>
> Alec Brown
>
> >
> > Signed-off-by: Michael Chang <mchang@suse.com>
> > ---
> > grub-core/commands/blsuki.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/grub-core/commands/blsuki.c b/grub-core/commands/blsuki.c
> > index 21d767f05..df25b6fbc 100644
> > --- a/grub-core/commands/blsuki.c
> > +++ b/grub-core/commands/blsuki.c
> > @@ -1510,6 +1510,12 @@ blsuki_is_default_entry (const char *def_entry, grub_blsuki_entry_t *entry,
> > int
> > return true;
> >
> > def_idx = grub_strtol (def_entry, &def_entry_end, 0);
> > + if (grub_errno == GRUB_ERR_BAD_NUMBER)
> > + {
> > + grub_errno = GRUB_ERR_NONE;
> > + return false;
> > + }
> > +
> > if (*def_entry_end != '\0' || def_idx < 0 || def_idx > GRUB_INT_MAX)
> > return false;
> >
> > --
> > 2.51.1
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://urldefense.com/v3/__https://lists.gnu.org/mailman/listinfo/grub-
> > devel__;!!ACWV5N9M2RV99hQ!PT07VdaZpBxwaIforN0iXRL1OG2vC6xOV2SQ6cgn5eVQvHxGFpEtx3c9eNW_ApT4iml6Fnoj
> > FNjVxarvCu1C$
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] blsuki: Fix grub_errno leakage in blsuki_is_default_entry()
2025-11-10 7:38 Michael Chang via Grub-devel
` (2 preceding siblings ...)
2025-11-10 17:26 ` Alec Brown via Grub-devel
@ 2025-11-11 4:21 ` Michael Chang via Grub-devel
3 siblings, 0 replies; 10+ messages in thread
From: Michael Chang via Grub-devel @ 2025-11-11 4:21 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: Michael Chang
Hi,
Sorry. Forgot to mention the downstream bug report which was fixed by
the patch.
https://bugzilla.suse.com/show_bug.cgi?id=1253097
Thanks,
Michael
On Mon, Nov 10, 2025 at 03:38:13PM +0800, Michael Chang wrote:
> The grub_strtol() call in blsuki_is_default_entry() can set grub_errno
> to GRUB_ERR_BAD_NUMBER if the input string cannot be converted into any
> valid digits.
>
> This errno value is currently left uncleared, which can lead to
> unexpected behavior in subsequent logic that tests the result of a
> function by checking grub_errno.
>
> Clear grub_errno and return false when GRUB_ERR_BAD_NUMBER is set, as
> this specific error should be ignored in this context.
>
> Signed-off-by: Michael Chang <mchang@suse.com>
> ---
> grub-core/commands/blsuki.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/grub-core/commands/blsuki.c b/grub-core/commands/blsuki.c
> index 21d767f05..df25b6fbc 100644
> --- a/grub-core/commands/blsuki.c
> +++ b/grub-core/commands/blsuki.c
> @@ -1510,6 +1510,12 @@ blsuki_is_default_entry (const char *def_entry, grub_blsuki_entry_t *entry, int
> return true;
>
> def_idx = grub_strtol (def_entry, &def_entry_end, 0);
> + if (grub_errno == GRUB_ERR_BAD_NUMBER)
> + {
> + grub_errno = GRUB_ERR_NONE;
> + return false;
> + }
> +
> if (*def_entry_end != '\0' || def_idx < 0 || def_idx > GRUB_INT_MAX)
> return false;
>
> --
> 2.51.1
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 10+ messages in thread