| Message ID | 20241105221149.507119-1-jh80.chung@samsung.com |
|---|---|
| State | New |
| Headers | show |
| Series | boot: image-board: Mismatch a type between variable and return value | expand |
On 11/5/24 11:11 PM, Jaehoon Chung wrote: > phys_addr_t can be used unsigned long long type with CONFIG_PHYS_64BIT. > But hextoul() is always returning to unsigned long. It can be assigned > to unexpected value. To avoid wrong behavior, change from hextoul() to > simple_strtoull(). > > Fixes: a4df06e41fa2 ("boot: fdt: Change type of env_get_bootm_low() to phys_addr_t") Looking at: 7e5f460ec457 ("global: Convert simple_strtoul() with hex to hextoul()") do you have to update any of the other hextoul() calls too ? Maybe we need hextoull() ? > Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> > --- > boot/image-board.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/boot/image-board.c b/boot/image-board.c > index 1757e5816d84..977343a8995c 100644 > --- a/boot/image-board.c > +++ b/boot/image-board.c > @@ -547,7 +547,7 @@ int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start, > /* a value of "no" or a similar string will act like 0, > * turning the "load high" feature off. This is intentional. > */ > - initrd_high = hextoul(s, NULL); > + initrd_high = simple_strtoull(s, NULL, 16); > if (initrd_high == ~0) > initrd_copy_to_ram = 0; > } else {
> -----Original Message----- > From: Marek Vasut <marek.vasut@mailbox.org> > Sent: Wednesday, November 6, 2024 7:57 AM > To: Jaehoon Chung <jh80.chung@samsung.com>; u-boot@lists.denx.de > Cc: trini@konsulko.com; sjg@chromium.org; marek.vasut+renesas@mailbox.org; > laurent.pinchart@ideasonboard.com; m.szyprowski@samsung.com > Subject: Re: [PATCH] boot: image-board: Mismatch a type between variable and return value > > On 11/5/24 11:11 PM, Jaehoon Chung wrote: > > phys_addr_t can be used unsigned long long type with CONFIG_PHYS_64BIT. > > But hextoul() is always returning to unsigned long. It can be assigned > > to unexpected value. To avoid wrong behavior, change from hextoul() to > > simple_strtoull(). > > > > Fixes: a4df06e41fa2 ("boot: fdt: Change type of env_get_bootm_low() to phys_addr_t") > > Looking at: > > 7e5f460ec457 ("global: Convert simple_strtoul() with hex to hextoul()") Thanks for informing it. > > do you have to update any of the other hextoul() calls too ? I didn't check other hextoul(). During booting with initramfs on RPi (32bit), I found the problem that is not working fine because of this. > > Maybe we need hextoull() ? IMO, We need hextoull() for some cases. If there is no objection, I will send the patch for adding hextoull(). And I will check entire cases. Best Regards, Jaehoon Chung > > > Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> > > --- > > boot/image-board.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/boot/image-board.c b/boot/image-board.c > > index 1757e5816d84..977343a8995c 100644 > > --- a/boot/image-board.c > > +++ b/boot/image-board.c > > @@ -547,7 +547,7 @@ int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start, > > /* a value of "no" or a similar string will act like 0, > > * turning the "load high" feature off. This is intentional. > > */ > > - initrd_high = hextoul(s, NULL); > > + initrd_high = simple_strtoull(s, NULL, 16); > > if (initrd_high == ~0) > > initrd_copy_to_ram = 0; > > } else { >
On 11/6/24 12:47 AM, Jaehoon Chung wrote: > >> -----Original Message----- >> From: Marek Vasut <marek.vasut@mailbox.org> >> Sent: Wednesday, November 6, 2024 7:57 AM >> To: Jaehoon Chung <jh80.chung@samsung.com>; u-boot@lists.denx.de >> Cc: trini@konsulko.com; sjg@chromium.org; marek.vasut+renesas@mailbox.org; >> laurent.pinchart@ideasonboard.com; m.szyprowski@samsung.com >> Subject: Re: [PATCH] boot: image-board: Mismatch a type between variable and return value >> >> On 11/5/24 11:11 PM, Jaehoon Chung wrote: >>> phys_addr_t can be used unsigned long long type with CONFIG_PHYS_64BIT. >>> But hextoul() is always returning to unsigned long. It can be assigned >>> to unexpected value. To avoid wrong behavior, change from hextoul() to >>> simple_strtoull(). >>> >>> Fixes: a4df06e41fa2 ("boot: fdt: Change type of env_get_bootm_low() to phys_addr_t") >> >> Looking at: >> >> 7e5f460ec457 ("global: Convert simple_strtoul() with hex to hextoul()") > > Thanks for informing it. > >> >> do you have to update any of the other hextoul() calls too ? > > I didn't check other hextoul(). > During booting with initramfs on RPi (32bit), I found the problem that is not working fine because of this. Please check the other sites too, maybe there are more of these issues lurking around. >> Maybe we need hextoull() ? > > IMO, We need hextoull() for some cases. > If there is no objection, I will send the patch for adding hextoull(). > And I will check entire cases. No objection from me. Thank you.
diff --git a/boot/image-board.c b/boot/image-board.c index 1757e5816d84..977343a8995c 100644 --- a/boot/image-board.c +++ b/boot/image-board.c @@ -547,7 +547,7 @@ int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start, /* a value of "no" or a similar string will act like 0, * turning the "load high" feature off. This is intentional. */ - initrd_high = hextoul(s, NULL); + initrd_high = simple_strtoull(s, NULL, 16); if (initrd_high == ~0) initrd_copy_to_ram = 0; } else {
phys_addr_t can be used unsigned long long type with CONFIG_PHYS_64BIT. But hextoul() is always returning to unsigned long. It can be assigned to unexpected value. To avoid wrong behavior, change from hextoul() to simple_strtoull(). Fixes: a4df06e41fa2 ("boot: fdt: Change type of env_get_bootm_low() to phys_addr_t") Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> --- boot/image-board.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)