On Fri, Jan 02, 2026 at 07:20:06PM +0530, Avnish Chouhan wrote: > 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!
You are welcome! > 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. Cool! > > > + 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! Great! > > > + 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? Please check range and drop "grub_errno != GRUB_ERR_NONE" check. Daniel _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
