Re: [PATCH] Fix FORTIFY_SOURCE false positive

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2023-10-03 09:22, Volker Weißmann wrote:
> 
> On 02.10.23 20:00, Siddhesh Poyarekar wrote:
>>
>> 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?
> 
> This is my first patch I sent to a mailing list, so forgive me if the
> formatting is wrong:

No worries, please review the Contribution checklist[1], which goes into 
much detail about submitting a patch to glibc.

One issue is, if you reply to a patch with a patch like you've done now, 
patchwork[2] does not pick it up; it is what we use to manage patches. 
Please fix the below review comments and send a v3.  Basically in your 
git repo, commit your patch with the subject and body of this email as 
the git commit log (with the Signed-off-by) and then generate a patch 
like so:

git format-patch --subject-prefix="PATCH v3" -1

and then send that patch to the list using "git send-email".

> 
> 
> 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.
> 
> Signed-off-by: Volker Weißmann <volker.weissmann@gmx.de>
> ---
>   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
> +          into this if clause here. */

Please replace this code in the comment with a descriptive line as 
Adhemerval suggested, something like "Process has reached the maximum 
number of open files."

> +          || errno == EMFILE)
>       return 1;
>         return -1;
>       }
> -- 
> 2.42.0
>>
>> Thanks,
>> Sid
> 


Thanks,
Sid

[1] https://sourceware.org/glibc/wiki/Contribution%20checklist
[2] https://patchwork.sourceware.org/project/glibc/list/