Re: [patches] dl: RELRO handling crashes when kernel enforces MPROTECT restrictions

[Maxim Kuvyrkov <maxim@xxxxxxxxxxxxxxxx>]

On Feb 26, 2011, at 8:18 PM, Pierre Ynard wrote:

> When dlopen'ing a library that needs to make the stack executable,
> the RELRO section is made writable again to modify the __stack_prot
> variable. However, the return value of the mprotect() call is not
> checked; so if mprotect() fails, instead of gracefully handling the
> error, the dynamic loader tries to write to __stack_prot anyway, which
> results in a segmentation fault. And this mprotect() call *will* fail on
> PaX kernels that enforce restrictions on it.
> 
> The simple fix is to check the return value and simply fail to load the
> problematic library, instead of crashing the whole process. And it's
> just good programming practice.
> 
> This patch was submitted to glibc but unfortunately rejected for the
> reason: "Just use a supported kernel." For more history, please see
> glibc bug http://sources.redhat.com/bugzilla/show_bug.cgi?id=12492 and
> Debian bug http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=611195

The patch is OK, I'll check it in shortly.

From functional perspective all the patch does is adding a gracious error path to the dynamic linker.  Ulrich's decision to keep this patch out of GLIBC seems overly conservative.

Thank you,

--
Maxim Kuvyrkov
Mentor Graphics / CodeSourcery
+7-812-677-6839



> 
> 
> --- elf/dl-load.c	2011-01-26 22:02:02.000000000 +0100
> +++ elf/dl-load.c	2011-01-26 22:30:22.000000000 +0100
> @@ -1398,7 +1398,11 @@
> 	  if (__builtin_expect (p + s <= relro_end, 1))
> 	    {
> 	      /* The variable lies in the region protected by RELRO.  */
> -	      __mprotect ((void *) p, s, PROT_READ|PROT_WRITE);
> +	      if (__mprotect ((void *) p, s, PROT_READ|PROT_WRITE) < 0)
> +		{
> +		  errstring = N_("cannot change memory protections");
> +		  goto call_lose_errno;
> +		}
> 	      __stack_prot |= PROT_READ|PROT_WRITE|PROT_EXEC;
> 	      __mprotect ((void *) p, s, PROT_READ);
> 	    }
> 
> 
> Regards,
> 
> -- 
> Pierre Ynard
> "Une âme dans un corps, c'est comme un dessin sur une feuille de papier."