* [PATCH] efi: check *path non-null before grub_strrchr
@ 2014-11-05 12:53 Leif Lindholm
2014-11-05 14:33 ` Andrei Borzenkov
0 siblings, 1 reply; 6+ messages in thread
From: Leif Lindholm @ 2014-11-05 12:53 UTC (permalink / raw)
To: grub-devel
The EFI version of grub_machine_get_bootlocation crops the boot image
name back to the last / in order to get a directory path. However, it
does not check that *name is actually set before calling grub_strrchr
to do this, and neither does grub_strrchr before dereferencing a NULL
pointer.
Parent function, grub_set_prefix_and_root, does check the pointer
before using.
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
grub-core/kern/efi/init.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
index 942ab02..e9c85de 100644
--- a/grub-core/kern/efi/init.c
+++ b/grub-core/kern/efi/init.c
@@ -63,10 +63,13 @@ grub_machine_get_bootlocation (char **device, char
**path)
if (!*device && grub_efi_net_config)
grub_efi_net_config (image->device_handle, device, path);
- /* Get the directory. */
- p = grub_strrchr (*path, '/');
- if (p)
- *p = '\0';
+ if (*path)
+ {
+ /* Get the directory. */
+ p = grub_strrchr (*path, '/');
+ if (p)
+ *p = '\0';
+ }
}
void
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] efi: check *path non-null before grub_strrchr
2014-11-05 12:53 [PATCH] efi: check *path non-null before grub_strrchr Leif Lindholm
@ 2014-11-05 14:33 ` Andrei Borzenkov
2014-11-05 14:52 ` Leif Lindholm
0 siblings, 1 reply; 6+ messages in thread
From: Andrei Borzenkov @ 2014-11-05 14:33 UTC (permalink / raw)
To: The development of GNU GRUB
On Wed, Nov 5, 2014 at 3:53 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> The EFI version of grub_machine_get_bootlocation crops the boot image
> name back to the last / in order to get a directory path. However, it
> does not check that *name is actually set before calling grub_strrchr
> to do this, and neither does grub_strrchr before dereferencing a NULL
> pointer.
>
I wonder - do you actually have firmware that returns empty path?
> Parent function, grub_set_prefix_and_root, does check the pointer
> before using.
>
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
> grub-core/kern/efi/init.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
> index 942ab02..e9c85de 100644
> --- a/grub-core/kern/efi/init.c
> +++ b/grub-core/kern/efi/init.c
> @@ -63,10 +63,13 @@ grub_machine_get_bootlocation (char **device, char
> **path)
> if (!*device && grub_efi_net_config)
> grub_efi_net_config (image->device_handle, device, path);
>
> - /* Get the directory. */
> - p = grub_strrchr (*path, '/');
> - if (p)
> - *p = '\0';
> + if (*path)
> + {
> + /* Get the directory. */
> + p = grub_strrchr (*path, '/');
> + if (p)
> + *p = '\0';
> + }
> }
>
> void
> --
> 1.7.10.4
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] efi: check *path non-null before grub_strrchr
2014-11-05 14:33 ` Andrei Borzenkov
@ 2014-11-05 14:52 ` Leif Lindholm
2014-11-05 15:19 ` Andrei Borzenkov
0 siblings, 1 reply; 6+ messages in thread
From: Leif Lindholm @ 2014-11-05 14:52 UTC (permalink / raw)
To: The development of GNU GRUB
On Wed, Nov 05, 2014 at 05:33:40PM +0300, Andrei Borzenkov wrote:
> On Wed, Nov 5, 2014 at 3:53 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > The EFI version of grub_machine_get_bootlocation crops the boot image
> > name back to the last / in order to get a directory path. However, it
> > does not check that *name is actually set before calling grub_strrchr
> > to do this, and neither does grub_strrchr before dereferencing a NULL
> > pointer.
> >
>
> I wonder - do you actually have firmware that returns empty path?
I did (internal development version), and that's being fixed that end
too, but more graceful error handling in GRUB would still be nice.
/
Leif
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] efi: check *path non-null before grub_strrchr
2014-11-05 14:52 ` Leif Lindholm
@ 2014-11-05 15:19 ` Andrei Borzenkov
2014-11-07 14:19 ` Leif Lindholm
0 siblings, 1 reply; 6+ messages in thread
From: Andrei Borzenkov @ 2014-11-05 15:19 UTC (permalink / raw)
To: The development of GNU GRUB
On Wed, Nov 5, 2014 at 5:52 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, Nov 05, 2014 at 05:33:40PM +0300, Andrei Borzenkov wrote:
>> On Wed, Nov 5, 2014 at 3:53 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> > The EFI version of grub_machine_get_bootlocation crops the boot image
>> > name back to the last / in order to get a directory path. However, it
>> > does not check that *name is actually set before calling grub_strrchr
>> > to do this, and neither does grub_strrchr before dereferencing a NULL
>> > pointer.
>> >
>>
>> I wonder - do you actually have firmware that returns empty path?
>
> I did (internal development version), and that's being fixed that end
> too, but more graceful error handling in GRUB would still be nice.
>
Sure. I wish we could display some meaningful warning here, but it is
too early at this stage. If get_loaded_image fails to return proper
image path, booting is probably screwed anyway.
You have commit access, right?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] efi: check *path non-null before grub_strrchr
2014-11-05 15:19 ` Andrei Borzenkov
@ 2014-11-07 14:19 ` Leif Lindholm
2014-11-15 17:00 ` Andrei Borzenkov
0 siblings, 1 reply; 6+ messages in thread
From: Leif Lindholm @ 2014-11-07 14:19 UTC (permalink / raw)
To: The development of GNU GRUB
On Wed, Nov 05, 2014 at 06:19:16PM +0300, Andrei Borzenkov wrote:
> On Wed, Nov 5, 2014 at 5:52 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Wed, Nov 05, 2014 at 05:33:40PM +0300, Andrei Borzenkov wrote:
> >> On Wed, Nov 5, 2014 at 3:53 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >> > The EFI version of grub_machine_get_bootlocation crops the boot image
> >> > name back to the last / in order to get a directory path. However, it
> >> > does not check that *name is actually set before calling grub_strrchr
> >> > to do this, and neither does grub_strrchr before dereferencing a NULL
> >> > pointer.
> >> >
> >>
> >> I wonder - do you actually have firmware that returns empty path?
> >
> > I did (internal development version), and that's being fixed that end
> > too, but more graceful error handling in GRUB would still be nice.
> >
>
> Sure. I wish we could display some meaningful warning here, but it is
> too early at this stage. If get_loaded_image fails to return proper
> image path, booting is probably screwed anyway.
Indeed, but with this patch you would at least get a confused error
message later on.
> You have commit access, right?
Yes - clear to push?
/
Leif
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] efi: check *path non-null before grub_strrchr
2014-11-07 14:19 ` Leif Lindholm
@ 2014-11-15 17:00 ` Andrei Borzenkov
0 siblings, 0 replies; 6+ messages in thread
From: Andrei Borzenkov @ 2014-11-15 17:00 UTC (permalink / raw)
To: Leif Lindholm; +Cc: The development of GNU GRUB
В Fri, 7 Nov 2014 14:19:58 +0000
Leif Lindholm <leif.lindholm@linaro.org> пишет:
> On Wed, Nov 05, 2014 at 06:19:16PM +0300, Andrei Borzenkov wrote:
> > On Wed, Nov 5, 2014 at 5:52 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > > On Wed, Nov 05, 2014 at 05:33:40PM +0300, Andrei Borzenkov wrote:
> > >> On Wed, Nov 5, 2014 at 3:53 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > >> > The EFI version of grub_machine_get_bootlocation crops the boot image
> > >> > name back to the last / in order to get a directory path. However, it
> > >> > does not check that *name is actually set before calling grub_strrchr
> > >> > to do this, and neither does grub_strrchr before dereferencing a NULL
> > >> > pointer.
> > >> >
> > >>
> > >> I wonder - do you actually have firmware that returns empty path?
> > >
> > > I did (internal development version), and that's being fixed that end
> > > too, but more graceful error handling in GRUB would still be nice.
> > >
> >
> > Sure. I wish we could display some meaningful warning here, but it is
> > too early at this stage. If get_loaded_image fails to return proper
> > image path, booting is probably screwed anyway.
>
> Indeed, but with this patch you would at least get a confused error
> message later on.
>
> > You have commit access, right?
>
> Yes - clear to push?
>
Yes. It is clear bug fix.
> /
> Leif
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-11-15 17:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-05 12:53 [PATCH] efi: check *path non-null before grub_strrchr Leif Lindholm
2014-11-05 14:33 ` Andrei Borzenkov
2014-11-05 14:52 ` Leif Lindholm
2014-11-05 15:19 ` Andrei Borzenkov
2014-11-07 14:19 ` Leif Lindholm
2014-11-15 17:00 ` Andrei Borzenkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).