public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v4] newlib: libc: Fix crash on fprintf to a wide-oriented stream.
@ 2023-11-08 12:04 Takashi Yano
  2023-11-08 19:40 ` Corinna Vinschen
  0 siblings, 1 reply; 3+ messages in thread
From: Takashi Yano @ 2023-11-08 12:04 UTC (permalink / raw)
  To: newlib; +Cc: Takashi Yano, Corinna Vinschen

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(-)

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;
-- 
2.39.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] newlib: libc: Fix crash on fprintf to a wide-oriented stream.
  2023-11-08 12:04 [PATCH v4] newlib: libc: Fix crash on fprintf to a wide-oriented stream Takashi Yano
@ 2023-11-08 19:40 ` Corinna Vinschen
  2023-11-08 22:00   ` Takashi Yano
  0 siblings, 1 reply; 3+ messages in thread
From: Corinna Vinschen @ 2023-11-08 19:40 UTC (permalink / raw)
  To: Takashi Yano; +Cc: newlib

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] newlib: libc: Fix crash on fprintf to a wide-oriented stream.
  2023-11-08 19:40 ` Corinna Vinschen
@ 2023-11-08 22:00   ` Takashi Yano
  0 siblings, 0 replies; 3+ messages in thread
From: Takashi Yano @ 2023-11-08 22:00 UTC (permalink / raw)
  To: newlib

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.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-11-08 22:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-08 12:04 [PATCH v4] newlib: libc: Fix crash on fprintf to a wide-oriented stream Takashi Yano
2023-11-08 19:40 ` Corinna Vinschen
2023-11-08 22:00   ` Takashi Yano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).