Re: [PATCH v5] newlib: libc: Fix crash on fprintf to a wide-oriented stream.

[Corinna Vinschen <vinschen@redhat.com>]

On Nov  9 19:26, Takashi Yano wrote:
> On Thu, 9 Nov 2023 11:08:03 +0100
> Corinna Vinschen wrote:
> > On Nov  9 06:48, Takashi Yano wrote:
> > > Previously, fprintf() on a wide-oriented stream crashes or outputs
> > > garbage. This is because a narrow char string which can be odd bytes
> > > in length is cast into a wide char string which should be even
> > > bytes in length in __sprint_r/__sfputs_r based on the __SWID flag.
> > > As a result, if the length is odd bytes, the reading buffer runs over
> > > the buffer length, which causes a crash. If the length is even bytes,
> > > garbage is printed.
> > > 
> > > With this patch, any output to the stream which is set to different
> > > orientation fails with error just like glibc. Note that it behaves
> > > differently from other libc implementations such as BSD, musl and
> > > Solaris.
> > > 
> > > Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
> > > Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
> > > ---
> > >  newlib/libc/stdio/fgetwc.c    |  6 ++++--
> > >  newlib/libc/stdio/fgetwc_u.c  |  3 ++-
> > >  newlib/libc/stdio/fgetws.c    |  3 ++-
> > >  newlib/libc/stdio/fputs.c     |  9 ++++++---
> > >  newlib/libc/stdio/fputwc.c    |  6 ++++--
> > >  newlib/libc/stdio/fputwc_u.c  |  3 ++-
> > >  newlib/libc/stdio/fputws.c    |  6 ++++--
> > >  newlib/libc/stdio/fread.c     |  7 ++++++-
> > >  newlib/libc/stdio/fwrite.c    |  9 +++++++--
> > >  newlib/libc/stdio/local.h     | 31 +++++++++++++++++--------------
> > >  newlib/libc/stdio/putc.c      |  4 ++++
> > >  newlib/libc/stdio/puts.c      |  9 ++++++---
> > >  newlib/libc/stdio/refill.c    |  3 ++-
> > >  newlib/libc/stdio/ungetc.c    |  6 +++++-
> > >  newlib/libc/stdio/ungetwc.c   |  5 +++--
> > >  newlib/libc/stdio/vfprintf.c  |  5 ++++-
> > >  newlib/libc/stdio/vfscanf.c   |  6 +++++-
> > >  newlib/libc/stdio/vfwprintf.c |  5 ++++-
> > >  newlib/libc/stdio/vfwscanf.c  |  6 +++++-
> > >  19 files changed, 92 insertions(+), 40 deletions(-)
> > 
> > Looks good, please push.
> 
> Thanks. Should this also be applied to cygwin-3_4-branch?

Tricky question.  It's a bugfix, yeah, but a bugfix for an undefined
situation.  And it's also a behavioral change.  So, from my POV we
shouldn't backport it.

But if you have another POV, we can discuss it.  It occured to me that
you didn't mention where the testcase is coming from.  Was that a
real-world problem?  If so, where and in which circumstances?


Corinna