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

Takashi Yano <[email protected]>
Newsgroups gmane.comp.lib.newlib
Message-ID <[email protected]>
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 <[email protected]>
v2-0001-newlib-libc-Fix-crash-on-fprintf-to-a-wide-orient.patch (text/plain, 7.8 KB)
From e755461cd26805482f062fcc5db174c4b779adf9 Mon Sep 17 00:00:00 2001
From: Takashi Yano <[email protected]>
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 <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.