Re: [PATCH v3] Fix FORTIFY_SOURCE false positive

[Florian Weimer <fweimer@redhat.com>]

* Volker Weißmann:

> When -D_FORTIFY_SOURCE=2 was given during compilation,
> sprintf and similar functions will check if their
> first argument is in read-only memory and exit with
> *** %n in writable segment detected ***
> otherwise. To check if the memory is read-only, glibc
> reads frpm the file "/proc/self/maps". If opening this
> file fails due to too many open files (EMFILE), glibc
> will now ignore this error.
>
> Fixes [BZ #30932]
>
> Signed-off-by: Volker Weißmann <volker.weissmann@gmx.de>
> ---
>  sysdeps/unix/sysv/linux/readonly-area.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/sysdeps/unix/sysv/linux/readonly-area.c b/sysdeps/unix/sysv/linux/readonly-area.c
> index edc68873f6..ba32372ebb 100644
> --- a/sysdeps/unix/sysv/linux/readonly-area.c
> +++ b/sysdeps/unix/sysv/linux/readonly-area.c
> @@ -42,7 +42,9 @@ __readonly_area (const char *ptr, size_t size)
>  	     to the /proc filesystem if it is set[ug]id.  There has
>  	     been no willingness to change this in the kernel so
>  	     far.  */
> -	  || errno == EACCES)
> +	  || errno == EACCES
> +	  /* Process has reached the maximum number of open files.  */
> +	  || errno == EMFILE)
>  	return 1;
>        return -1;
>      }

This whole thing is rather questionable.

First of all, the compiler should detect the fact that a format argument
to printf is a string literal and record that in the flags argument
(which already exists for __printf_chk).  Then we wouldn't have to do
any %n security checks for most uses of %n.  (The flags argument cannot
be spoofed just by altering the string.)

Siddhesh, is that something you could be working on?

Even without that, if we are willing to trust the ld.so data structures,
we can do the permission check in userspace for strings that come from
.rodata.  We an find the ELF object that contains them and check if the
loadable segment has the right permissions (or we are in the RELRO
area).

After these changes, I think we can fail hard on /proc-related errors
because they are very unlikely to happen.

Thanks,
Florian