Patchwork libmudflap: check for NULL path in dlopen wrapper

login
register
mail settings
Submitter Matthias Klose
Date Jan. 16, 2013, 3:32 p.m.
Message ID <50F6C809.3070601@ubuntu.com>
Download mbox | patch
Permalink /patch/212561/
State New
Headers show

Comments

Matthias Klose - Jan. 16, 2013, 3:32 p.m.
From the bug report:

If mudflap is used to instrument a program using dlopen, and the program
(assuming it is compiled with -rdynamic) loads itself by passing NULL for the
path to dlopen, the program will crash unconditionally; that is, regardless of
the options passed to mudflap, so long as instrumentation is enabled.

This is because (at least with GNU/Linux) it is valid to pass a NULL pointer as
the path argument to dlopen, and the instrumentation code unconditionally uses
strlen on that pointer, without checking first if it is NULL.

Ok for the trunk?

  Matthias
Jakub Jelinek - Jan. 16, 2013, 3:38 p.m.
On Wed, Jan 16, 2013 at 04:32:25PM +0100, Matthias Klose wrote:
> 	PR mudflap/24619
> 	* mf-hooks2.c (dlopen wrapper): Check for NULL path.
> 
> Index: b/src/libmudflap/mf-hooks2.c
> ===================================================================
> --- a/libmudflap/mf-hooks2.c
> +++ b/libmudflap/mf-hooks2.c
> @@ -1677,8 +1677,10 @@
>    size_t n;
>    TRACE ("%s\n", __PRETTY_FUNCTION__);
>    n = strlen (path);
> -  MF_VALIDATE_EXTENT (path, CLAMPADD(n, 1), __MF_CHECK_READ, "dlopen path");
> -  p = dlopen (path, flags);
> +  if (NULL != path) {
> +    MF_VALIDATE_EXTENT (path, CLAMPADD(n, 1), __MF_CHECK_READ, "dlopen path");
> +    p = dlopen (path, flags);
> +  }

That can't be the right fix, given you still do strlen (path)
unconditionally.  Thus the compiler can assume path is non-NULL.

	Jakub

Patch


	PR mudflap/24619
	* mf-hooks2.c (dlopen wrapper): Check for NULL path.

Index: b/src/libmudflap/mf-hooks2.c
===================================================================
--- a/libmudflap/mf-hooks2.c
+++ b/libmudflap/mf-hooks2.c
@@ -1677,8 +1677,10 @@ 
   size_t n;
   TRACE ("%s\n", __PRETTY_FUNCTION__);
   n = strlen (path);
-  MF_VALIDATE_EXTENT (path, CLAMPADD(n, 1), __MF_CHECK_READ, "dlopen path");
-  p = dlopen (path, flags);
+  if (NULL != path) {
+    MF_VALIDATE_EXTENT (path, CLAMPADD(n, 1), __MF_CHECK_READ, "dlopen path");
+    p = dlopen (path, flags);
+  }
   if (NULL != p) {
 #ifdef MF_REGISTER_dlopen
     __mf_register (p, 0, MF_REGISTER_dlopen, "dlopen result");