[PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty

[PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty

[John Paul Adrian Glaubitz]

We have recently backported the upstream changes to Debian's grub2 package
to support modern SPARC hardware for Debian's sparc64 port.

One of the changes introduced a regression on systems with NVME devices
but not 

    The of_path_of_nvme function (commit 2391d57, ieee1275: add nvme
    support within ofpath) introduced a functional regression:
    
    On systems where of_path is empty, i.e. non-OpenFirmware
    machines, grub-probe crashes in of_path_of_nvme when trying
    to append the disk name to an empty of_path.

[PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty

[John Paul Adrian Glaubitz]

The of_path_of_nvme function (commit 2391d57, ieee1275: add nvme
support within ofpath) introduced a functional regression:

On systems where of_path is empty, i.e. non-OpenFirmware
machines, grub-probe crashes in of_path_of_nvme when trying
to append the disk name to an empty of_path.

Signed-off-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
---
 grub-core/osdep/linux/ofpath.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
index 1c30e7233..da0000f0f 100644
--- a/grub-core/osdep/linux/ofpath.c
+++ b/grub-core/osdep/linux/ofpath.c
@@ -389,8 +389,13 @@ of_path_of_nvme(const char *sys_devname __attribute__((unused)),
     }
 
   of_path = find_obppath (sysfs_path);
+
+  if(of_path)
+    strcat (of_path, disk);
+  else
+    of_path = strdup(disk);
+
   free (sysfs_path);
-  strcat (of_path, disk);
   return of_path;
 }
 
-- 
2.16.2

Re: [PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty

[John Paul Adrian Glaubitz]

On 03/01/2018 04:47 PM, John Paul Adrian Glaubitz wrote:
> We have recently backported the upstream changes to Debian's grub2 package
> to support modern SPARC hardware for Debian's sparc64 port.
> 
> One of the changes introduced a regression on systems with NVME devices
> but not

Sorry, that message was accidentally sent incompletely. I will re-send the
patch with a proper message. Please don't merge.

Adrian

-- 
  .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Bug#891773: Info received ([PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty)

[Debian Bug Tracking System]

Thank you for the additional information you have supplied regarding
this Bug report.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 GRUB Maintainers <pkg-grub-devel@lists.alioth.debian.org>

If you wish to submit further information on this problem, please
send it to 891773@bugs.debian.org.

Please do not send mail to owner@bugs.debian.org unless you wish
to report a problem with the Bug-tracking system.

-- 
891773: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891773
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems

Re: [PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty

[Daniel Kiper]

On Thu, Mar 01, 2018 at 08:10:10PM -0700, Eric Snowberg wrote:
> > On Mar 1, 2018, at 3:34 PM, John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> wrote:
> >
> > The of_path_of_nvme function (commit 2391d57, ieee1275: add nvme
> > support within ofpath) introduced a functional regression:
> >
> > On systems which are not based on Open Firmware but have at
> > least one NVME device, find_obppath will return NULL and thus
> > trying to append the disk name to of_path will result in a
> > crash.
> >
> > The proper behavior of of_path_of_nvme is, however, to just
> > return NULL in such cases, like other users of find_obppath,
> > such as of_path_of_scsi.
> >
> > Signed-off-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> > ---
> > grub-core/osdep/linux/ofpath.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
> > index 1c30e7233..61806212e 100644
> > --- a/grub-core/osdep/linux/ofpath.c
> > +++ b/grub-core/osdep/linux/ofpath.c
> > @@ -389,8 +389,11 @@ of_path_of_nvme(const char *sys_devname __attribute__((unused)),
> >     }
> >
> >   of_path = find_obppath (sysfs_path);
> > +
> > +  if (of_path)
> > +    strcat (of_path, disk);
> > +
> >   free (sysfs_path);
> > -  strcat (of_path, disk);
> >   return of_path;
> > }
> >
> > --
> > 2.16.2
>
> Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com>

Applied. Please CC me next time.

Daniel

Re: [PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty

[Eric Snowberg]

> On Mar 1, 2018, at 3:34 PM, John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> wrote:
> 
> The of_path_of_nvme function (commit 2391d57, ieee1275: add nvme
> support within ofpath) introduced a functional regression:
> 
> On systems which are not based on Open Firmware but have at
> least one NVME device, find_obppath will return NULL and thus
> trying to append the disk name to of_path will result in a
> crash.
> 
> The proper behavior of of_path_of_nvme is, however, to just
> return NULL in such cases, like other users of find_obppath,
> such as of_path_of_scsi.
> 
> Signed-off-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> ---
> grub-core/osdep/linux/ofpath.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
> index 1c30e7233..61806212e 100644
> --- a/grub-core/osdep/linux/ofpath.c
> +++ b/grub-core/osdep/linux/ofpath.c
> @@ -389,8 +389,11 @@ of_path_of_nvme(const char *sys_devname __attribute__((unused)),
>     }
> 
>   of_path = find_obppath (sysfs_path);
> +
> +  if (of_path)
> +    strcat (of_path, disk);
> +
>   free (sysfs_path);
> -  strcat (of_path, disk);
>   return of_path;
> }
> 
> -- 
> 2.16.2

Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com>

[PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty

[John Paul Adrian Glaubitz]

The of_path_of_nvme function (commit 2391d57, ieee1275: add nvme
support within ofpath) introduced a functional regression:

On systems which are not based on Open Firmware but have at
least one NVME device, find_obppath will return NULL and thus
trying to append the disk name to of_path will result in a
crash.

The proper behavior of of_path_of_nvme is, however, to just
return NULL in such cases, like other users of find_obppath,
such as of_path_of_scsi.

Signed-off-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
---
 grub-core/osdep/linux/ofpath.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
index 1c30e7233..61806212e 100644
--- a/grub-core/osdep/linux/ofpath.c
+++ b/grub-core/osdep/linux/ofpath.c
@@ -389,8 +389,11 @@ of_path_of_nvme(const char *sys_devname __attribute__((unused)),
     }
 
   of_path = find_obppath (sysfs_path);
+
+  if (of_path)
+    strcat (of_path, disk);
+
   free (sysfs_path);
-  strcat (of_path, disk);
   return of_path;
 }
 
-- 
2.16.2

[PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty

[John Paul Adrian Glaubitz]

The of_path_of_nvme function (commit 2391d57, ieee1275: add nvme
support within ofpath) introduced a functional regression:

On systems which are not based on Open Firmware but have at
least one NVME device, find_obppath will return an empty path
and appending the disk name to of_path will therefore result
in a crash. Thus, when of_path is empty, just return the
disk name in of_path_of_nvme.

Signed-off-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
---
 grub-core/osdep/linux/ofpath.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
index 1c30e7233..da0000f0f 100644
--- a/grub-core/osdep/linux/ofpath.c
+++ b/grub-core/osdep/linux/ofpath.c
@@ -389,8 +389,13 @@ of_path_of_nvme(const char *sys_devname __attribute__((unused)),
     }
 
   of_path = find_obppath (sysfs_path);
+
+  if(of_path)
+    strcat (of_path, disk);
+  else
+    of_path = strdup(disk);
+
   free (sysfs_path);
-  strcat (of_path, disk);
   return of_path;
 }
 
-- 
2.16.2