Re: fprintf() crashes on wide-oriented stream.

[Takashi Yano <takashi.yano@nifty.ne.jp>]

[-- Attachment #1: Type: text/plain, Size: 1690 bytes --]

On Tue, 26 Sep 2023 12:41:47 +0900
Takashi Yano wrote:
> Hi,
> 
> I noticed that the following test case crashes at printf() with current
> newlib.
> 
> 
> #include <stdio.h>
> #include <wchar.h>
> #include <locale.h>
> 
> int main()
> {
> 	setlocale(LC_ALL, "C.UTF-8");
> 	wprintf(L"%ls\n", L"aaaa"); /* or fwide(stdout, 1); */
> 	printf("%ls\n", L"bbbb");  /* <--- crash here */
> 	return 0;
> }
> 
> 
> I looked into this problem and found the cause.
> 
> 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, crash does not happen, but garbage
> is printed. This hapens if printf("%ls\r\n", L"bbbb"); is used instead.
>                                       ^^
> 
> The same issue seemed to be reported ten years ago.
> https://sourceware.org/pipermail/newlib/2013/010831.html
> 
> I have built a patch attached for this issue.
> 
> With this patch, __sfputs_r/__sprint_r is split into two versions, one
> is for vfprintf which does not handle wide string, and the other (newly
> introduced __sfputws_r/__swprin_r) is for vfwprintf which handles wide
> string. Please note that fprintf gets working for wide orient stream
> just like BSD libc, which behaves differently from GNU libc.
> 
> This patch also fixes nano-vfprintf.c as well as vfprintf.c/vfwprintf.c
> in the same manner.

v2: Remove __sprint_r from nano-vfprintf.c which does not seem to be used
    anymore.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: v2-0001-newlib-libc-Fix-crash-on-fprintf-to-a-wide-orient.patch --]
[-- Type: text/plain, Size: 7993 bytes --]

From e755461cd26805482f062fcc5db174c4b779adf9 Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Tue, 26 Sep 2023 08:19:07 +0900
Subject: [PATCH v2] newlib: libc: Fix crash on fprintf to a wide-oriented
 stream.

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, __sfputs_r/__sprint_r is split into two versions;
one is for vfprintf which does not handle wide string, and the other
(newly introduced __sfputws_r/__swprin_r) is for vfwprintf which
handles wide string. Please note that fprintf() gets working for
wide-oriented stream just like BSD libc, which behaves differently
from GNU libc.

Revewed-by:
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
---
 newlib/libc/stdio/nano-vfprintf.c | 72 ++-------------------------
 newlib/libc/stdio/vfprintf.c      | 44 ++---------------
 newlib/libc/stdio/vfwprintf.c     | 81 ++++++++++++++++++++++++++++++-
 3 files changed, 88 insertions(+), 109 deletions(-)

diff --git a/newlib/libc/stdio/nano-vfprintf.c b/newlib/libc/stdio/nano-vfprintf.c
index 0d42a940f..ffcb1df62 100644
--- a/newlib/libc/stdio/nano-vfprintf.c
+++ b/newlib/libc/stdio/nano-vfprintf.c
@@ -340,54 +340,6 @@ err:
   return EOF;
 }
 #else
-/* As __ssputs_r, __sprint_r is used by output functions for wide char,
-   like vfwprint.  */
-/* Flush out all the vectors defined by the given uio,
-   then reset it so that it can be reused.  */
-int
-__sprint_r (struct _reent *ptr,
-       FILE *fp,
-       register struct __suio *uio)
-{
-  register int err = 0;
-
-  if (uio->uio_resid == 0)
-    {
-      uio->uio_iovcnt = 0;
-      return 0;
-    }
-#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
-    if (fp->_flags2 & __SWID)
-      {
-	struct __siov *iov;
-	wchar_t *p;
-	int i, len;
-
-	iov = uio->uio_iov;
-	for (; uio->uio_resid != 0;
-	     uio->uio_resid -= len * sizeof (wchar_t), iov++)
-	  {
-	    p = (wchar_t *) iov->iov_base;
-	    len = iov->iov_len / sizeof (wchar_t);
-	    for (i = 0; i < len; i++)
-	      {
-		if (_fputwc_r (ptr, p[i], fp) == WEOF)
-		  {
-		    err = -1;
-		    goto out;
-		  }
-	      }
-	  }
-	}
-      else
-#endif
-	err = __sfvwrite_r(ptr, fp, uio);
-out:
-  uio->uio_resid = 0;
-  uio->uio_iovcnt = 0;
-  return err;
-}
-
 _NOINLINE_STATIC int
 __sfputc_r (struct _reent *ptr,
        int c,
@@ -407,27 +359,11 @@ __sfputs_r (struct _reent *ptr,
 {
   register int i;
 
-#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
-  if (fp->_flags2 & __SWID)
-    {
-      wchar_t *p;
-
-      p = (wchar_t *) buf;
-      for (i = 0; i < (len / sizeof (wchar_t)); i++)
-	{
-	  if (_fputwc_r (ptr, p[i], fp) == WEOF)
-	    return -1;
-	}
-    }
-  else
-#endif
+  for (i = 0; i < len; i++)
     {
-      for (i = 0; i < len; i++)
-	{
-	  /* Call __sfputc_r to skip _fputc_r.  */
-	  if (__sfputc_r (ptr, (int)buf[i], fp) == EOF)
-	    return -1;
-	}
+      /* Call __sfputc_r to skip _fputc_r.  */
+      if (__sfputc_r (ptr, (int)buf[i], fp) == EOF)
+	return -1;
     }
   return (0);
 }
diff --git a/newlib/libc/stdio/vfprintf.c b/newlib/libc/stdio/vfprintf.c
index 6a198e2c6..95412ced9 100644
--- a/newlib/libc/stdio/vfprintf.c
+++ b/newlib/libc/stdio/vfprintf.c
@@ -187,7 +187,7 @@ static char *rcsid = "$Id$";
 # endif
 #endif
 
-/* The __sprint_r/__ssprint_r functions are shared between all versions of
+/* The __ssputs_r/__ssprint_r functions are shared between all versions of
    vfprintf and vfwprintf.  They must only be defined once, which we do in
    the INTEGER_ONLY versions here. */
 #ifdef STRING_ONLY
@@ -370,23 +370,9 @@ __sfputs_r (struct _reent *ptr,
 {
 	register int i;
 
-#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
-	if (fp->_flags2 & __SWID) {
-		wchar_t *p;
-
-		p = (wchar_t *) buf;
-		for (i = 0; i < (len / sizeof (wchar_t)); i++) {
-			if (_fputwc_r (ptr, p[i], fp) == WEOF)
-				return -1;
-		}
-	} else {
-#else
-	{
-#endif
-		for (i = 0; i < len; i++) {
-			if (_fputc_r (ptr, buf[i], fp) == EOF)
-				return -1;
-		}
+	for (i = 0; i < len; i++) {
+		if (_fputc_r (ptr, buf[i], fp) == EOF)
+			return -1;
 	}
 	return (0);
 }
@@ -406,27 +392,7 @@ __sprint_r (struct _reent *ptr,
 		uio->uio_iovcnt = 0;
 		return (0);
 	}
-#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
-	if (fp->_flags2 & __SWID) {
-		struct __siov *iov;
-		wchar_t *p;
-		int i, len;
-
-		iov = uio->uio_iov;
-		for (; uio->uio_resid != 0;
-		     uio->uio_resid -= len * sizeof (wchar_t), iov++) {
-			p = (wchar_t *) iov->iov_base;
-			len = iov->iov_len / sizeof (wchar_t);
-			for (i = 0; i < len; i++) {
-				if (_fputwc_r (ptr, p[i], fp) == WEOF) {
-					err = -1;
-					goto out;
-				}
-			}
-		}
-	} else
-#endif
-		err = __sfvwrite_r(ptr, fp, uio);
+	err = __sfvwrite_r(ptr, fp, uio);
 out:
 	uio->uio_resid = 0;
 	uio->uio_iovcnt = 0;
diff --git a/newlib/libc/stdio/vfwprintf.c b/newlib/libc/stdio/vfwprintf.c
index 7807a1229..de494f44a 100644
--- a/newlib/libc/stdio/vfwprintf.c
+++ b/newlib/libc/stdio/vfwprintf.c
@@ -155,18 +155,95 @@ int _VFWPRINTF_R (struct _reent *, FILE *, const wchar_t *, va_list);
 # ifdef STRING_ONLY
 #  define __SPRINT __ssprint_r
 # else
-#  define __SPRINT __sprint_r
+#  define __SPRINT __swprint_r
 # endif
 int __SPRINT (struct _reent *, FILE *, register struct __suio *);
 #else
 # ifdef STRING_ONLY
 #  define __SPRINT __ssputs_r
 # else
-#  define __SPRINT __sfputs_r
+#  define __SPRINT __sfputws_r
 # endif
 int __SPRINT (struct _reent *, FILE *, const char *, size_t);
 #endif
+
 #ifndef STRING_ONLY
+#ifdef INTEGER_ONLY
+#ifndef _FVWRITE_IN_STREAMIO
+int
+__sfputws_r (struct _reent *ptr,
+       FILE *fp,
+       const char *buf,
+       size_t len)
+{
+	register int i;
+
+#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
+	wchar_t *p;
+
+	p = (wchar_t *) buf;
+	for (i = 0; i < (len / sizeof (wchar_t)); i++) {
+		if (_fputwc_r (ptr, p[i], fp) == WEOF)
+			return -1;
+	}
+#else
+	for (i = 0; i < len; i++) {
+		if (_fputc_r (ptr, buf[i], fp) == EOF)
+			return -1;
+	}
+#endif
+	return (0);
+}
+#endif
+/*
+ * Flush out all the vectors defined by the given uio,
+ * then reset it so that it can be reused.
+ */
+int
+__swprint_r (struct _reent *ptr,
+       FILE *fp,
+       register struct __suio *uio)
+{
+	register int err = 0;
+
+	if (uio->uio_resid == 0) {
+		uio->uio_iovcnt = 0;
+		return (0);
+	}
+#if defined _WIDE_ORIENT && (!defined _ELIX_LEVEL || _ELIX_LEVEL >= 4)
+	do {
+		struct __siov *iov;
+		wchar_t *p;
+		int i, len;
+
+		iov = uio->uio_iov;
+		for (; uio->uio_resid != 0;
+			 uio->uio_resid -= len * sizeof (wchar_t), iov++) {
+			p = (wchar_t *) iov->iov_base;
+			len = iov->iov_len / sizeof (wchar_t);
+			for (i = 0; i < len; i++) {
+				if (_fputwc_r (ptr, p[i], fp) == WEOF) {
+					err = -1;
+					goto out;
+				}
+			}
+		}
+	} while (0);
+#else
+	err = __sfvwrite_r(ptr, fp, uio);
+#endif
+out:
+	uio->uio_resid = 0;
+	uio->uio_iovcnt = 0;
+	return (err);
+}
+#else /* !INTEGER_ONLY */
+#ifndef _FVWRITE_IN_STREAMIO
+int __sfputws_r (struct _reent *, FILE *, const char *buf, size_t);
+#endif
+int __swprint_r (struct _reent *, FILE *, register struct __suio *);
+#endif /* !INTEGER_ONLY */
+
 #ifdef _UNBUF_STREAM_OPT
 /*
  * Helper function for `fprintf to unbuffered unix file': creates a
-- 
2.39.0