Re: [PATCH v3 6/7] Use PRINTF_FORTIFY instead of _IO_FLAGS2_FORTIFY (bug 11319)

["Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>]

On Wed, 19 Dec 2018, Szabolcs Nagy wrote:
>
>i would just do
>
>if (maxlen == -1)
>  .. old code ..
>else
>  .. new code ..
>
>to revert the old sprintf behaviour.

I found that just checking for maxlen == -1 is not enough for reverting to
the old behavior, because maxlen could be -1 when __vsprintf_internal is
called from __sprintf_chk and __vsprintf_chk (and not only  from sprintf
and vsprintf).  Thus, I added a new macro that __sprintf_chk and
__vsprintf_chk can set (through mode_flags), to let __vsprintf_internal
know where the call came from, then restore the old behavior.

More important to notice is the fact that the overwriting of the
destination buffer is *not* the only behavior affected by the refactoring.
Before the refactoring, sprintf and vsprintf would use _IO_str_jumps,
whereas __sprintf_chk and __vsprintf_chk would use _IO_str_chk_jumps.
After the refactoring, all use _IO_str_chk_jumps, which would make
sprintf and vsprintf report buffer overflows and terminate the program,
which sounds wrong to me (I haven't noticed this effect on sprintf and
vsprintf before, sorry about that. Maybe I should add an extra test to
avoid something similar in the future).

With the new macro, I can install the same jump table that used to be
installed before the refactoring, thus fully reverting to the old
behaviour.  I have just submitted a new patch at:

https://sourceware.org/ml/libc-alpha/2018-12/msg00838.html

>later the code may be refactored further (it should be
>possible to use shared code between sprintf, snprintf
>and sprintf_chk, but that requires a lot more changes).

It wasn't my intention to suggest that sprintf and snprintf should share
code (although I'm not against it)...  I was just referring to *sprintf,
__*sprintf_chk, and __vsprintf_internal.  Perhaps, I did not understand
what you mean with this comment.