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

Message ID 20231108120403.643-1-takashi.yano@nifty.ne.jp
State New
Headers
Series [v4] newlib: libc: Fix crash on fprintf to a wide-oriented stream. |

Commit Message

Takashi Yano Nov. 8, 2023, 12:04 p.m. UTC
  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     | 30 ++++++++++++++++--------------
 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, 91 insertions(+), 40 deletions(-)
  

Comments

Corinna Vinschen Nov. 8, 2023, 7:40 p.m. UTC | #1
Hi Takashi,

On Nov  8 21:04, Takashi Yano wrote:
> --- a/newlib/libc/stdio/local.h
> +++ b/newlib/libc/stdio/local.h
> @@ -231,21 +231,23 @@ extern _READ_WRITE_RETURN_TYPE __swrite64 (struct _reent *, void *,
>   * Set the orientation for a stream. If o > 0, the stream has wide-
>   * orientation. If o < 0, the stream has byte-orientation.
>   */
> -#define ORIENT(fp,ori)					\
> -  do								\
> -    {								\
> -      if (!((fp)->_flags & __SORD))	\
> -	{							\
> -	  (fp)->_flags |= __SORD;				\
> -	  if (ori > 0)						\
> -	    (fp)->_flags2 |= __SWID;				\
> -	  else							\
> -	    (fp)->_flags2 &= ~__SWID;				\
> -	}							\
> -    }								\
> -  while (0)
> +#define ORIENT(fp,ori)			\
> +  (					\
> +    (					\
> +      ((fp)->_flags & __SORD) ?		\
> +	0				\
> +      :					\
> +	(				\

You're missing to set (fp)->_flags |= __SORD here.

> +	  (ori > 0) ?			\
> +	    ((fp)->_flags2 |= __SWID)	\
> +	  :				\
> +	    ((fp)->_flags2 &= ~__SWID)	\
> +	)				\
> +    ),					\
> +    (((fp)->_flags2 & __SWID) ? 1 : -1)	\
> +  )



Thanks,
Corinna
  
Takashi Yano Nov. 8, 2023, 10 p.m. UTC | #2
On Wed, 8 Nov 2023 20:40:57 +0100
Corinna Vinschen wrote:
> On Nov  8 21:04, Takashi Yano wrote:
> > --- a/newlib/libc/stdio/local.h
> > +++ b/newlib/libc/stdio/local.h
> > +#define ORIENT(fp,ori)			\
> > +  (					\
> > +    (					\
> > +      ((fp)->_flags & __SORD) ?		\
> > +	0				\
> > +      :					\
> > +	(				\
> 
> You're missing to set (fp)->_flags |= __SORD here.
> 
> > +	  (ori > 0) ?			\
> > +	    ((fp)->_flags2 |= __SWID)	\
> > +	  :				\
> > +	    ((fp)->_flags2 &= ~__SWID)	\
> > +	)				\
> > +    ),					\
> > +    (((fp)->_flags2 & __SWID) ? 1 : -1)	\
> > +  )

Argh! Thanks.
  

Patch

diff --git a/newlib/libc/stdio/fgetwc.c b/newlib/libc/stdio/fgetwc.c
index 522697e9b..38e2db897 100644
--- a/newlib/libc/stdio/fgetwc.c
+++ b/newlib/libc/stdio/fgetwc.c
@@ -177,8 +177,10 @@  _fgetwc_r (struct _reent *ptr,
   wint_t r;
 
   _newlib_flockfile_start (fp);
-  ORIENT(fp, 1);
-  r = __fgetwc (ptr, fp);
+  if (ORIENT(fp, 1) != 1)
+    r = WEOF;
+  else
+    r = __fgetwc (ptr, fp);
   _newlib_flockfile_end (fp);
   return r;
 }
diff --git a/newlib/libc/stdio/fgetwc_u.c b/newlib/libc/stdio/fgetwc_u.c
index cbb97360b..007f9c844 100644
--- a/newlib/libc/stdio/fgetwc_u.c
+++ b/newlib/libc/stdio/fgetwc_u.c
@@ -33,7 +33,8 @@  wint_t
 _fgetwc_unlocked_r (struct _reent *ptr,
 	register FILE *fp)
 {
-  ORIENT(fp, 1);
+  if (ORIENT(fp, 1) != 1)
+    return WEOF;
   return __fgetwc (ptr, fp);
 }
 
diff --git a/newlib/libc/stdio/fgetws.c b/newlib/libc/stdio/fgetws.c
index c942806c6..3b7fa9516 100644
--- a/newlib/libc/stdio/fgetws.c
+++ b/newlib/libc/stdio/fgetws.c
@@ -110,7 +110,8 @@  _fgetws_r (struct _reent *ptr,
   unsigned char *nl;
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, 1);
+  if (ORIENT (fp, 1) != 1)
+    goto error;
 
   if (n <= 0)
     {
diff --git a/newlib/libc/stdio/fputs.c b/newlib/libc/stdio/fputs.c
index 4e9cb7547..a4f18df2a 100644
--- a/newlib/libc/stdio/fputs.c
+++ b/newlib/libc/stdio/fputs.c
@@ -103,8 +103,10 @@  _fputs_r (struct _reent * ptr,
   CHECK_INIT(ptr, fp);
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
-  result = __sfvwrite_r (ptr, fp, &uio);
+  if (ORIENT (fp, -1) != -1)
+    result = EOF;
+  else
+    result = __sfvwrite_r (ptr, fp, &uio);
   _newlib_flockfile_end (fp);
   return result;
 #else
@@ -113,7 +115,8 @@  _fputs_r (struct _reent * ptr,
   CHECK_INIT(ptr, fp);
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    goto error;
   /* Make sure we can write.  */
   if (cantwrite (ptr, fp))
     goto error;
diff --git a/newlib/libc/stdio/fputwc.c b/newlib/libc/stdio/fputwc.c
index 9a4e80d52..ef2be1048 100644
--- a/newlib/libc/stdio/fputwc.c
+++ b/newlib/libc/stdio/fputwc.c
@@ -169,8 +169,10 @@  _fputwc_r (struct _reent *ptr,
   wint_t r;
 
   _newlib_flockfile_start (fp);
-  ORIENT(fp, 1);
-  r = __fputwc(ptr, wc, fp);
+  if (ORIENT(fp, 1) != 1)
+    r = WEOF;
+  else
+    r = __fputwc(ptr, wc, fp);
   _newlib_flockfile_end (fp);
   return r;
 }
diff --git a/newlib/libc/stdio/fputwc_u.c b/newlib/libc/stdio/fputwc_u.c
index d4e51532a..a5cc9a5d5 100644
--- a/newlib/libc/stdio/fputwc_u.c
+++ b/newlib/libc/stdio/fputwc_u.c
@@ -34,7 +34,8 @@  _fputwc_unlocked_r (struct _reent *ptr,
 	wchar_t wc,
 	FILE *fp)
 {
-  ORIENT(fp, 1);
+  if (ORIENT(fp, 1) != 1)
+    return WEOF;
   return __fputwc(ptr, wc, fp);
 }
 
diff --git a/newlib/libc/stdio/fputws.c b/newlib/libc/stdio/fputws.c
index 92f2cbf6a..a9ac9d6d7 100644
--- a/newlib/libc/stdio/fputws.c
+++ b/newlib/libc/stdio/fputws.c
@@ -105,7 +105,8 @@  _fputws_r (struct _reent *ptr,
   struct __siov iov;
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, 1);
+  if (ORIENT (fp, 1) != 1)
+    goto error;
   if (cantwrite (ptr, fp) != 0)
     goto error;
   uio.uio_iov = &iov;
@@ -129,7 +130,8 @@  error:
   return (-1);
 #else
   _newlib_flockfile_start (fp);
-  ORIENT (fp, 1);
+  if (ORIENT (fp, 1) != 1)
+    goto error;
   if (cantwrite (ptr, fp) != 0)
     goto error;
 
diff --git a/newlib/libc/stdio/fread.c b/newlib/libc/stdio/fread.c
index df8321461..8664dc3e5 100644
--- a/newlib/libc/stdio/fread.c
+++ b/newlib/libc/stdio/fread.c
@@ -158,7 +158,11 @@  _fread_r (struct _reent * ptr,
   CHECK_INIT(ptr, fp);
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    {
+      count = 0;
+      goto ret;
+    }
   if (fp->_r < 0)
     fp->_r = 0;
   total = resid;
@@ -252,6 +256,7 @@  _fread_r (struct _reent * ptr,
       return crlf_r(ptr, fp, buf, total, 0) / size;
     }
 #endif
+ret:
   _newlib_flockfile_end (fp);
   return count;
 }
diff --git a/newlib/libc/stdio/fwrite.c b/newlib/libc/stdio/fwrite.c
index d499f6f59..2d9749693 100644
--- a/newlib/libc/stdio/fwrite.c
+++ b/newlib/libc/stdio/fwrite.c
@@ -133,7 +133,11 @@  _fwrite_r (struct _reent * ptr,
   CHECK_INIT(ptr, fp);
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    {
+      _newlib_flockfile_exit (fp);
+      return 0;
+    }
   if (__sfvwrite_r (ptr, fp, &uio) == 0)
     {
       _newlib_flockfile_exit (fp);
@@ -148,7 +152,8 @@  _fwrite_r (struct _reent * ptr,
   CHECK_INIT (ptr, fp);
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    goto ret;
   /* Make sure we can write.  */
   if (cantwrite (ptr, fp))
     goto ret;
diff --git a/newlib/libc/stdio/local.h b/newlib/libc/stdio/local.h
index b34c7c9d8..0e1d4bbf8 100644
--- a/newlib/libc/stdio/local.h
+++ b/newlib/libc/stdio/local.h
@@ -231,21 +231,23 @@  extern _READ_WRITE_RETURN_TYPE __swrite64 (struct _reent *, void *,
  * Set the orientation for a stream. If o > 0, the stream has wide-
  * orientation. If o < 0, the stream has byte-orientation.
  */
-#define ORIENT(fp,ori)					\
-  do								\
-    {								\
-      if (!((fp)->_flags & __SORD))	\
-	{							\
-	  (fp)->_flags |= __SORD;				\
-	  if (ori > 0)						\
-	    (fp)->_flags2 |= __SWID;				\
-	  else							\
-	    (fp)->_flags2 &= ~__SWID;				\
-	}							\
-    }								\
-  while (0)
+#define ORIENT(fp,ori)			\
+  (					\
+    (					\
+      ((fp)->_flags & __SORD) ?		\
+	0				\
+      :					\
+	(				\
+	  (ori > 0) ?			\
+	    ((fp)->_flags2 |= __SWID)	\
+	  :				\
+	    ((fp)->_flags2 &= ~__SWID)	\
+	)				\
+    ),					\
+    (((fp)->_flags2 & __SWID) ? 1 : -1)	\
+  )
 #else
-#define ORIENT(fp,ori)
+#define ORIENT(fp,ori) (-1)
 #endif
 
 /* WARNING: _dcvt is defined in the stdlib directory, not here!  */
diff --git a/newlib/libc/stdio/putc.c b/newlib/libc/stdio/putc.c
index 6a410e216..925341939 100644
--- a/newlib/libc/stdio/putc.c
+++ b/newlib/libc/stdio/putc.c
@@ -84,6 +84,8 @@  _putc_r (struct _reent *ptr,
 {
   int result;
   CHECK_INIT (ptr, fp);
+  if (ORIENT (fp, -1) != -1)
+    return EOF;
   _newlib_flockfile_start (fp);
   result = __sputc_r (ptr, c, fp);
   _newlib_flockfile_end (fp);
@@ -100,6 +102,8 @@  putc (int c,
   struct _reent *reent = _REENT;
 
   CHECK_INIT (reent, fp);
+  if (ORIENT (fp, -1) != -1)
+    return EOF;
   _newlib_flockfile_start (fp);
   result = __sputc_r (reent, c, fp);
   _newlib_flockfile_end (fp);
diff --git a/newlib/libc/stdio/puts.c b/newlib/libc/stdio/puts.c
index d4d7b7ffb..20d889b81 100644
--- a/newlib/libc/stdio/puts.c
+++ b/newlib/libc/stdio/puts.c
@@ -87,8 +87,10 @@  _puts_r (struct _reent *ptr,
   fp = _stdout_r (ptr);
   CHECK_INIT (ptr, fp);
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
-  result = (__sfvwrite_r (ptr, fp, &uio) ? EOF : '\n');
+  if (ORIENT (fp, -1) != -1)
+    result = EOF;
+  else
+    result = (__sfvwrite_r (ptr, fp, &uio) ? EOF : '\n');
   _newlib_flockfile_end (fp);
   return result;
 #else
@@ -100,7 +102,8 @@  _puts_r (struct _reent *ptr,
   fp = _stdout_r (ptr);
   CHECK_INIT (ptr, fp);
   _newlib_flockfile_start (fp);
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    goto err;
   /* Make sure we can write.  */
   if (cantwrite (ptr, fp))
     goto err;
diff --git a/newlib/libc/stdio/refill.c b/newlib/libc/stdio/refill.c
index 7bd38806c..c1ef7e120 100644
--- a/newlib/libc/stdio/refill.c
+++ b/newlib/libc/stdio/refill.c
@@ -43,7 +43,8 @@  __srefill_r (struct _reent * ptr,
 
   CHECK_INIT (ptr, fp);
 
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    return EOF;
 
   fp->_r = 0;			/* largely a convenience for callers */
 
diff --git a/newlib/libc/stdio/ungetc.c b/newlib/libc/stdio/ungetc.c
index 533c28eac..79914af08 100644
--- a/newlib/libc/stdio/ungetc.c
+++ b/newlib/libc/stdio/ungetc.c
@@ -125,7 +125,11 @@  _ungetc_r (struct _reent *rptr,
 
   _newlib_flockfile_start (fp);
 
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    {
+      _newlib_flockfile_exit (fp);
+      return EOF;
+    }
 
   /* After ungetc, we won't be at eof anymore */
   fp->_flags &= ~__SEOF;
diff --git a/newlib/libc/stdio/ungetwc.c b/newlib/libc/stdio/ungetwc.c
index 16d37f2d1..18636d773 100644
--- a/newlib/libc/stdio/ungetwc.c
+++ b/newlib/libc/stdio/ungetwc.c
@@ -82,8 +82,9 @@  _ungetwc_r (struct _reent *ptr,
   size_t len;
 
   _newlib_flockfile_start (fp);
-  ORIENT (fp, 1);
-  if (wc == WEOF)
+  if (ORIENT (fp, 1) != 1)
+    wc = WEOF;
+  else if (wc == WEOF)
     wc = WEOF;
   else if ((len = _wcrtomb_r(ptr, buf, wc, &fp->_mbstate)) == (size_t)-1)
     {
diff --git a/newlib/libc/stdio/vfprintf.c b/newlib/libc/stdio/vfprintf.c
index 6a198e2c6..42272abe4 100644
--- a/newlib/libc/stdio/vfprintf.c
+++ b/newlib/libc/stdio/vfprintf.c
@@ -845,7 +845,10 @@  _VFPRINTF_R (struct _reent *data,
 	CHECK_INIT (data, fp);
 	_newlib_flockfile_start (fp);
 
-	ORIENT(fp, -1);
+	if (ORIENT(fp, -1) != -1) {
+		_newlib_flockfile_exit (fp);
+		return (EOF);
+	}
 
 	/* sorry, fprintf(read_only_file, "") returns EOF, not 0 */
 	if (cantwrite (data, fp)) {
diff --git a/newlib/libc/stdio/vfscanf.c b/newlib/libc/stdio/vfscanf.c
index cfeea9876..71e2c3e22 100644
--- a/newlib/libc/stdio/vfscanf.c
+++ b/newlib/libc/stdio/vfscanf.c
@@ -589,7 +589,11 @@  __SVFSCANF_R (struct _reent *rptr,
 
   _newlib_flockfile_start (fp);
 
-  ORIENT (fp, -1);
+  if (ORIENT (fp, -1) != -1)
+    {
+      nassigned = EOF;
+      goto all_done;
+    }
 
   nassigned = 0;
   nread = 0;
diff --git a/newlib/libc/stdio/vfwprintf.c b/newlib/libc/stdio/vfwprintf.c
index 7807a1229..bbabbdaee 100644
--- a/newlib/libc/stdio/vfwprintf.c
+++ b/newlib/libc/stdio/vfwprintf.c
@@ -588,7 +588,10 @@  _VFWPRINTF_R (struct _reent *data,
 	CHECK_INIT (data, fp);
 	_newlib_flockfile_start (fp);
 
-	ORIENT(fp, 1);
+	if (ORIENT(fp, 1) != 1) {
+		_newlib_flockfile_exit (fp);
+		return (EOF);
+	}
 
 	/* sorry, fwprintf(read_only_file, "") returns EOF, not 0 */
 	if (cantwrite (data, fp)) {
diff --git a/newlib/libc/stdio/vfwscanf.c b/newlib/libc/stdio/vfwscanf.c
index df966f929..d2f91dde2 100644
--- a/newlib/libc/stdio/vfwscanf.c
+++ b/newlib/libc/stdio/vfwscanf.c
@@ -516,7 +516,11 @@  __SVFWSCANF_R (struct _reent *rptr,
 
   _newlib_flockfile_start (fp);
 
-  ORIENT (fp, 1);
+  if (ORIENT (fp, 1) != 1)
+    {
+      nassigned = EOF;
+      goto all_done;
+    }
 
   nassigned = 0;
   nread = 0;