On 2026-01-12 19:19, Daniel Kiper wrote:
On Mon, Jan 12, 2026 at 06:59:00PM +0530, Avnish Chouhan wrote:
Adding a check for invalid partition number. grub_strtoul() can fail
s/Adding/Add/
s/can/may/
in several scenarios like invalid input, overflow, etc will result in
s/etc/etc./
an invalid partition number which could lead to an undefined behavior.
s/will result in an invalid partition number which could lead to an
undefined behavior.
/Lack of proper check may lead to unexpected failures in the code
further./
Hi Daniel,
Thank you for the review!
I'll change these as suggested.
Signed-off-by: Avnish Chouhan <[email protected]>
---
grub-core/kern/ieee1275/openfw.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/grub-core/kern/ieee1275/openfw.c
b/grub-core/kern/ieee1275/openfw.c
index 3b492dd..f8ae515 100644
--- a/grub-core/kern/ieee1275/openfw.c
+++ b/grub-core/kern/ieee1275/openfw.c
@@ -512,7 +512,20 @@ grub_ieee1275_encode_devname (const char *path)
}
if (partition && partition[0])
{
- unsigned int partno = grub_strtoul (partition, 0, 0);
+ char *endptr;
+
+ unsigned long partno = grub_strtoul (partition, &endptr, 0);
+ grub_errno = GRUB_ERR_NONE;
I would do this
unsigned long partno;
char *endptr;
partno = grub_strtoul (partition, &endptr, 0);
grub_errno = GRUB_ERR_NONE;
Sure. I'll change like this.
+ if (*endptr != '\0' || partno > 65535 ||
+ (partno == 0 && ! grub_ieee1275_test_flag
(GRUB_IEEE1275_FLAG_0_BASED_PARTITIONS)))
+ {
+ grub_free (partition);
+ grub_free (device);
+ grub_free (encoding);
+ grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid partition
number"));
+ return NULL;
+ }
*optr++ = ',';
@@ -520,7 +533,7 @@ grub_ieee1275_encode_devname (const char *path)
/* GRUB partition 1 is OF partition 0. */
partno++;
- grub_snprintf (optr, sizeof ("XXXXXXXXXXXX"), "%d", partno);
+ grub_snprintf (optr, sizeof ("XXXXXXXXXXXX"), "%lu", partno);
Why is this change part of the patch? Even if it is valid it is
logically
different thing. And I would use "%zu" instead...
This we need due to change from "unsigned int partno" to "unsigned long
partno". In present code, partno is unsigned int, and we were using the
same (though in the present code, it is wrong. It must be '%u' instead
of '%d' anyway). Reason we didn't change this in earlier patch versions.
We'll change it to zu as suggested!
Regards,
Avnish Chouhan
Daniel
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
