Re: [PATCH] Fix FORTIFY_SOURCE false positive

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2023-10-02 11:53, Volker Weißmann wrote:
> 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 form the file "/proc/self/maps". If opening this
> file fails due to too many open files (EMFILE), glibc
> will now ignore this error.
> ---

Ugh, that looks like an easy way to defeat format string fortification :/

The fix is fine I think, just a little nit below.

>   sysdeps/unix/sysv/linux/readonly-area.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/readonly-area.c b/sysdeps/unix/sysv/linux/readonly-area.c
> index edc68873f6..629163461a 100644
> --- a/sysdeps/unix/sysv/linux/readonly-area.c
> +++ b/sysdeps/unix/sysv/linux/readonly-area.c
> @@ -42,7 +42,15 @@ __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
> +          /* Example code to trigger EMFILE:
> +          while(1) {
> +            FILE *file = fopen("/dev/zero", "r");
> +            assert(file != NULL);
> +          }
> +          If your libc was compiled with -D_FORTIFY_SOURCE=2, we run

Shouldn't this be "If the program was compiled with..." and not libc? 
Also, maybe the example code is unnecessary and you could just mention 
that the if the open file threshold is reached, this could become a 
spurious failure.

> +          into this if clause here. */
> +          || errno == EMFILE)
>   	return 1;
>         return -1;
>       }
> --
> 2.42.0
> 

Also, if you don't have a copyright assignment on file with the FSF, 
could you add a Signed-off-by to certify your contribution?

Thanks,
Sid