Re: [PATCH v2 7/8] Use PRINTF_FORTIFY instead of _IO_FLAGS2_FORTIFY.

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 29/10/2018 09:24, Gabriel F. T. Gomes wrote:
> On Mon, 29 Oct 2018, Gabriel F. T. Gomes wrote:
> 
>> From: Zack Weinberg <zackw@panix.com>
>>
>> int
>> -__vdprintf_chk (int d, int flags, const char *format, va_list arg)
>> +__vdprintf_chk (int d, int flag, const char *format, va_list ap)
>> {
>>
>> [...]
>>
>> -  done = __vfprintf_internal (&tmpfil.file, format, arg, 0);
>>
>> [...]
>>
>> -  return done;
>> +  return __vdprintf_internal (d, format, ap, mode);
> 
> While reviewing the first version of this patch, I noticed that it could
> have an effect on bug 20231, because, after this patch, __vdprintf_chk
> will call __vdprintf_internal, which has the extra check for EOF (as
> reported in the bug).  However, the bug is marked as unconfirmed, and I
> was unable to reproduce it with the following test case:
> 
>     #include <stdio.h>
>     #include <stdarg.h>
>     #include <sys/types.h>
>     #include <sys/stat.h>
>     #include <fcntl.h>
>     #include <unistd.h>
>     int
>     main (void)
>     {
>       int fd, libret, sysret;
>       va_list ap;
>       fd = open ("/tmp/blablabla", O_RDWR | O_TRUNC | O_CREAT, S_IRWXU);
>       if (fd == -1)
>         perror (NULL);
>       sysret = close (fd);
>       if (sysret)
>         perror (NULL);
>       libret = vdprintf (fd, "blablabla", ap);
>       if (libret != EOF)
>         printf ("Bug 20231 reproduced\n");
>       return 0;
>     }
> 
> Maybe the test case is wrong, in which case I could fix it, then mark this
> patch as solving bug 20231.  On the other hand, if the test case is
> correct, we could just close bug 20231.
> 

The BZ#20231 described issue does not happen, vdprintf_chk indeed returns EOF 
when trying to write on a closed file. It will currently call on master:

  _IO_new_file_seekoff
  \_ _IO_new_file_attach
     \_ __GI___vdprintf_chk
        \_ _IO_file_seek
           \_ __lseek64

And the __lseek64 will return -1/EBADF and thus _IO_new_file_attach will fail.

The code difference with default vdprintf is, in fact, BZ#11319, where vdprintf_chk 
does not return an error when an output error occurs (the same example reported on 
BZ#11319 fails with -D_FORTIFY_SOURCE=2 -O2).

I reopened BZ#11319 noting the fortified also needs fixing and the good thing is
the 'Add __v*printf_internal with flags arguments.' [1] refactor fixes it by
using a common correct function for both fortified and non-fortified vfprintf.
Please add a note that this patch fixes BZ#11319.

[1] https://sourceware.org/ml/libc-alpha/2018-10/msg00616.html