On 2025-12-20 19:04, Daniel Kiper wrote:
On Tue, Dec 02, 2025 at 06:29:44PM +0530, Avnish Chouhan wrote:
Adding a check for invalid partition number. grub_strtoul() can fail
in several scenarios like invalid input, overflow, etc will result in
an invalid partition number which could lead to an undefined behavior.
Signed-off-by: Avnish Chouhan <[email protected]>
---
grub-core/kern/ieee1275/openfw.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/grub-core/kern/ieee1275/openfw.c
b/grub-core/kern/ieee1275/openfw.c
index 3b492dd..e82dc34 100644
--- a/grub-core/kern/ieee1275/openfw.c
+++ b/grub-core/kern/ieee1275/openfw.c
@@ -512,7 +512,18 @@ grub_ieee1275_encode_devname (const char *path)
}
if (partition && partition[0])
{
- unsigned int partno = grub_strtoul (partition, 0, 0);
+ char *endptr;
+ grub_errno = GRUB_ERR_NONE;
You should do this reset after grub_strtoul() call. The commit
533cd4d68
(blsuki: Fix grub_errno leakage in blsuki_is_default_entry()) explains
why.
Hi Daniel,
Thank you so much for reviewing the patch!
This I have added so that we'll not catch any earlier errors.
I will reset grub_errno after we verify the grub_strtoul as you
suggested.
+ unsigned int partno = grub_strtoul (partition, &endptr, 0);
Do not cast result immediately to shorter type. You are not able to
detect overflow then. First assign result to type size equal to type
returned by the grub_strtoul() function and then check for overflows
properly.
Sure. I'll use unsigned long!
+ if (grub_errno != GRUB_ERR_NONE || *endptr != '\0')
This check is not reliable. Please take a look at the commit ac8a37dda
(net/http: Allow use of non-standard TCP/IP ports). It shows how it
should be done correctly. Even it is reverted now.
This I have added based on the Alec's suggestion in v1, same as you
suggested. I have used the same check as in the commit ac8a37dda, just
skipping the range check due to use of "grub_errno != GRUB_ERR_NONE"
condition.
Would you like me to add range check here?
By the way, it would be nice if you could verify correctness of
strtoul()/grub_strtoul()/... calls/checks in the GRUB code after
the release.
Sure Daniel, I will surly do!
Thank you!
Regards,
Avnish Chouhan
Daniel
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
