bugGNU GRUB - Bugs: bug #28422, [util/misc.c] realpath(path, NULL)...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #28422: [util/misc.c] realpath(path, NULL) leads to segfault

Submitted by:  Grégoire Sutre <gsutre>
Submitted on:  Mon 28 Dec 2009 12:56:47 AM UTC  
 
Category: CompilationSeverity: Major
Priority: 5 - NormalItem Group: Software Error
Status: FixedPrivacy: Public
Assigned to: NoneOriginator Name: 
Open/Closed: ClosedRelease: 
Release: Bazaar - trunkReproducibility: Every Time
Planned Release: None

(Jump to the original submission Jump to the original submission)

Mon 25 Jan 2010 09:13:10 AM UTC, comment #6:

Fixed in trunk by commits 2054 and 2128.

Grégoire Sutre <gsutre>
Project Member
Mon 28 Dec 2009 11:22:42 AM UTC, comment #5:

Hi Robert,

It's not bothering me at all, no problem :-)

The patch you sent almost works, there was just a missing #include <limits.h> (for PATH_MAX). I attach a new version of the patch with this include.

I looked at the rest of the code, and the only other place where realpath is used outside of an ifdef _linux_ is in util/ieee1275/ofpath.c, but there the result buffer is pre-allocated (with xmalloc (PATH_MAX)).

(file #19381)

Grégoire Sutre <gsutre>
Project Member
Mon 28 Dec 2009 02:22:35 AM UTC, comment #4:

Hi Grégoire,

Sorry to bother you, but after discussing this with Vladimir and thinking a bit more about it, I think we can live with an ifdef in our code if it's confined to util/misc.c and reasonably platform-independant.

Please let me know if attached patch works. Then for NetBSD support you'd only have to replace the remaining realpath() calls in other parts of the code.

Thanks

(file #19379)

Robert Millan <robertmh>
Project Administrator
Mon 28 Dec 2009 02:06:29 AM UTC, comment #3:

Yes, linking grub-mkrelpath with canonicalize-lgpl.c and malloca.c from Gnulib solves the problem.

Grégoire Sutre <gsutre>
Project Member
Mon 28 Dec 2009 01:36:45 AM UTC, comment #2:

Grégorie, could you please check if linking grub-mkrelpath with canonicalize-lgpl.c from Gnulib resolves your problem?

If so, I'll import it. It basically boils down to the same kludge, but I'd rather have it in external Gnulib code than in GRUB itself.

Best solution would be if someone could improve canonicalize-lgpl.c not to rely on PATH_MAX, however.

Robert Millan <robertmh>
Project Administrator
Mon 28 Dec 2009 01:22:36 AM UTC, comment #1:

This patch is inappropriate. It introduces arbitrary limit on pathlen even on platforms that aren't limited in this way (e.g. GNU/Hurd). If you make a patch with something like:
#ifdef _NetBSD_
p = xmalloc (PATH_MAX);
realpath (path, p);
#else
p = realpath (path, NULL);
#endif
Then we'll probably accept it

Vladimir Serbinenko <phcoder>
Project Administrator
Mon 28 Dec 2009 12:56:47 AM UTC, original submission:

Hi,

On NetBSD 5.0, grub-mkrelpath segfaults due to a call to realpath of the form realpath (path, NULL) in make_system_path_relative_to_its_root (file util/misc.c).

The following patch solves the problem, but a better option might be to use some autoconf macros?

Thanks for your work,

Gregoire

--- util/misc.c.orig 2009-12-28 01:09:18.000000000 +0100
+++ util/misc.c
@@ -52,6 +52,19 @@
#include <winioctl.h>
#endif

+/* Determine compile-time limit for realpath(). */
+#include <limits.h>
+#include <sys/param.h>
+#if defined(PATH_MAX)
+# define REALPATH_MAX PATH_MAX
+#elif defined(MAXPATHLEN)
+# define REALPATH_MAX MAXPATHLEN
+#else
+# warning "No compile-time limit found for realpath(). Using 1024."
+# define REALPATH_MAX 1024
+#endif
+
+
int verbosity = 0;

void
@@ -490,14 +503,10 @@ make_system_path_relative_to_its_root (c
size_t len;

/* canonicalize. */
- p = realpath (path, NULL);
-
- if (p == NULL)
+ p = (char *) xmalloc (REALPATH_MAX);
+ if (realpath (path, p) == NULL)
{
- if (errno != EINVAL)
- grub_util_error ("failed to get realpath of %s", path);
- else
- grub_util_error ("realpath not supporting (path, NULL)");
+ grub_util_error ("failed to get realpath of %s", path);
}
len = strlen (p) + 1;
buf = strdup (p);

Grégoire Sutre <gsutre>
Project Member

 

Attached Files
file #19381:  canonicalize_file_name-2.diff added by gsutre (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by robertmh (Posted a comment)
  • -unavailable- added by phcoder (Posted a comment)
  • -unavailable- added by gsutre (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 25 Jan 2010 09:13:10 AM UTCgsutreStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Mon 28 Dec 2009 11:22:42 AM UTCgsutreAttached File-=>Added canonicalize_file_name-2.diff, #19381
    Mon 28 Dec 2009 02:22:35 AM UTCrobertmhAttached File-=>Added canonicalize_file_name.diff, #19379

    Back to the top


    Powered by Savane 3.1-cleanup1