Bug#891773: [PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty
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 > > 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 > > --- > > 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 Applied. Please CC me next time. Daniel
Bug#891773: [PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty
> On Mar 1, 2018, at 3:34 PM, John Paul Adrian Glaubitz > 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 > --- > 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
Bug#891773: [PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty
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 --- 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
Bug#891773: [PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty
On 03/01/2018 05:59 PM, James Clarke wrote: Whitespace issues aside, should it not be returning NULL if of_path is NULL, like the other users of find_obppath, such as of_path_of_scsi? Hmm. Good point. This should restore the behaviour from before of_path_of_nvme was added, as grub_util_devname_to_ofpath would have previously returned NULL due to the unknown type? Right. I'll wait for some comments from Daniel and Eric first though. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Bug#891773: [PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty
On Thu, Mar 01, 2018 at 05:00:28PM +0100, John Paul Adrian Glaubitz 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 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 > --- > 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..daf0f 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); > + Whitespace issues aside, should it not be returning NULL if of_path is NULL, like the other users of find_obppath, such as of_path_of_scsi? This should restore the behaviour from before of_path_of_nvme was added, as grub_util_devname_to_ofpath would have previously returned NULL due to the unknown type? James >free (sysfs_path); > - strcat (of_path, disk); >return of_path; > } > > -- > 2.16.2
Bug#891773: [PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty
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 --- 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..daf0f 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
Bug#891773: [PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty
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 --- 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..daf0f 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
Bug#891773: [PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty
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.
Bug#891773: [PATCH] ieee1275: Fix crash in of_path_of_nvme when of_path is empty
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 - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
