git: 1f9c4610dde5 - releng/14.0 - fflush: correct buffer handling in __sflush

From: Ed Maste <emaste_at_FreeBSD.org>
Date: Wed, 08 Nov 2023 00:45:54 UTC
The branch releng/14.0 has been updated by emaste:

URL: https://cgit.FreeBSD.org/src/commit/?id=1f9c4610dde5ecda4ad219b19f16ec712bc1d793

commit 1f9c4610dde5ecda4ad219b19f16ec712bc1d793
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2023-08-03 15:08:03 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-11-08 00:45:25 +0000

    fflush: correct buffer handling in __sflush
    
    This fixes CVE-2014-8611 correctly.
    
    The commit that purported to fix CVE-2014-8611 (805288c2f062) only hid
    it behind another bug.  Two later commits, 86a16ada1ea6 and
    44cf1e5eb470, attempted to address this new bug but mostly just confused
    the issue.  This commit rolls back the three previous changes and fixes
    CVE-2014-8611 correctly.
    
    The key to understanding the bug (and the fix) is that `_w` has
    different meanings for different stream modes.  If the stream is
    unbuffered, it is always zero.  If the stream is fully buffered, it is
    the amount of space remaining in the buffer (equal to the buffer size
    when the buffer is empty and zero when the buffer is full).  If the
    stream is line-buffered, it is a negative number reflecting the amount
    of data in the buffer (zero when the buffer is empty and negative buffer
    size when the buffer is full).
    
    At the heart of `fflush()`, we call the stream's write function in a
    loop, where `t` represents the return value from the last call and `n`
    the amount of data that remains to be written.  When the write function
    fails, we need to move the unwritten data to the top of the buffer
    (unless nothing was written) and adjust `_p` (which points to the next
    free location in the buffer) and `_w` accordingly.  These variables have
    already been set to the values they should have after a successful
    flush, so instead of adjusting them down to reflect what was written,
    we're adjusting them up to reflect what remains.
    
    The bug was that while `_p` was always adjusted, we only adjusted `_w`
    if the stream was fully buffered.  The fix is to also adjust `_w` for
    line-buffered streams.  Everything else is just noise.
    
    Fixes: 805288c2f062
    Fixes: 86a16ada1ea6
    Fixes: 44cf1e5eb470
    Sponsored by:   Klara, Inc.
    
    (cherry picked from commit 1f90b4edffe815aebb35e74b79e10593b31f6b75)
    (cherry picked from commit 1e99535be2ea9c0ef8bc57fc885e9c01fa95d2dd)
    (cherry picked from commit d09a3bf72c0b5f1779c52269671872368c99f02a)
    (cherry picked from commit 92709431b14df6c0687446247ac57cfc189ee827)
    (cherry picked from commit 418f026bd5a5084c1c4e2e91ad38051f6caa928c)
    (cherry picked from commit abe12d2f4ce31c3da0961b1b0a58df11f5a41e19)
    (cherry picked from commit 4e0e01bf6511c28212d7dff94fe131a502e13026)
    (cherry picked from commit d2c65a1c948648f11342274029a3f18b90aa58d2)
    
    Approved by:    so
    Approved by:    re (implicit)
    Security:       FreeBSD-SA-23:15.stdio
    Sponsored by:   The FreeBSD Foundation
---
 lib/libc/stdio/fflush.c  | 27 ++++++++++-----------------
 lib/libc/stdio/fvwrite.c | 14 ++------------
 lib/libc/stdio/wbuf.c    | 12 ++----------
 3 files changed, 14 insertions(+), 39 deletions(-)

diff --git a/lib/libc/stdio/fflush.c b/lib/libc/stdio/fflush.c
index a7f9348def50..61f23335106f 100644
--- a/lib/libc/stdio/fflush.c
+++ b/lib/libc/stdio/fflush.c
@@ -103,11 +103,11 @@ __weak_reference(__fflush, fflush_unlocked);
 int
 __sflush(FILE *fp)
 {
-	unsigned char *p, *old_p;
-	int n, t, old_w;
+	unsigned char *p;
+	int n, f, t;
 
-	t = fp->_flags;
-	if ((t & __SWR) == 0)
+	f = fp->_flags;
+	if ((f & __SWR) == 0)
 		return (0);
 
 	if ((p = fp->_bf._base) == NULL)
@@ -119,26 +119,19 @@ __sflush(FILE *fp)
 	 * Set these immediately to avoid problems with longjmp and to allow
 	 * exchange buffering (via setvbuf) in user write function.
 	 */
-	old_p = fp->_p;
 	fp->_p = p;
-	old_w = fp->_w;
-	fp->_w = t & (__SLBF|__SNBF) ? 0 : fp->_bf._size;
+	fp->_w = f & (__SLBF|__SNBF) ? 0 : fp->_bf._size;
 
 	for (; n > 0; n -= t, p += t) {
 		t = _swrite(fp, (char *)p, n);
 		if (t <= 0) {
-			/* Reset _p and _w. */
-			if (p > fp->_p) {
+			if (p > fp->_p)
 				/* Some was written. */
 				memmove(fp->_p, p, n);
-				fp->_p += n;
-				if ((fp->_flags & (__SLBF | __SNBF)) == 0)
-					fp->_w -= n;
-			/* conditional to handle setvbuf */
-			} else if (p == fp->_p && errno == EINTR) {
-				fp->_p = old_p;
-				fp->_w = old_w;
-			}
+			/* Reset _p and _w. */
+			fp->_p += n;
+			if ((fp->_flags & __SNBF) == 0)
+				fp->_w -= n;
 			fp->_flags |= __SERR;
 			return (EOF);
 		}
diff --git a/lib/libc/stdio/fvwrite.c b/lib/libc/stdio/fvwrite.c
index 81e7ba89a644..acf8f72076cf 100644
--- a/lib/libc/stdio/fvwrite.c
+++ b/lib/libc/stdio/fvwrite.c
@@ -36,7 +36,6 @@
 static char sccsid[] = "@(#)fvwrite.c	8.1 (Berkeley) 6/4/93";
 #endif /* LIBC_SCCS and not lint */
 #include <sys/cdefs.h>
-#include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -53,7 +52,6 @@ int
 __sfvwrite(FILE *fp, struct __suio *uio)
 {
 	size_t len;
-	unsigned char *old_p;
 	char *p;
 	struct __siov *iov;
 	int w, s;
@@ -137,12 +135,8 @@ __sfvwrite(FILE *fp, struct __suio *uio)
 				COPY(w);
 				/* fp->_w -= w; */ /* unneeded */
 				fp->_p += w;
-				old_p = fp->_p;
-				if (__fflush(fp) == EOF) {
-					if (old_p == fp->_p && errno == EINTR)
-						fp->_p -= w;
+				if (__fflush(fp))
 					goto err;
-				}
 			} else if (len >= (w = fp->_bf._size)) {
 				/* write directly */
 				w = _swrite(fp, p, w);
@@ -181,12 +175,8 @@ __sfvwrite(FILE *fp, struct __suio *uio)
 				COPY(w);
 				/* fp->_w -= w; */
 				fp->_p += w;
-				old_p = fp->_p;
-				if (__fflush(fp) == EOF) {
-					if (old_p == fp->_p && errno == EINTR)
-						fp->_p -= w;
+				if (__fflush(fp))
 					goto err;
-				}
 			} else if (s >= (w = fp->_bf._size)) {
 				w = _swrite(fp, p, w);
 				if (w <= 0)
diff --git a/lib/libc/stdio/wbuf.c b/lib/libc/stdio/wbuf.c
index acbe379ad90e..558322b4001e 100644
--- a/lib/libc/stdio/wbuf.c
+++ b/lib/libc/stdio/wbuf.c
@@ -50,7 +50,6 @@ static char sccsid[] = "@(#)wbuf.c	8.1 (Berkeley) 6/4/93";
 int
 __swbuf(int c, FILE *fp)
 {
-	unsigned char *old_p;
 	int n;
 
 	/*
@@ -86,15 +85,8 @@ __swbuf(int c, FILE *fp)
 	}
 	fp->_w--;
 	*fp->_p++ = c;
-	old_p = fp->_p;
-	if (++n == fp->_bf._size || (fp->_flags & __SLBF && c == '\n')) {
-		if (__fflush(fp) != 0) {
-			if (fp->_p == old_p && errno == EINTR) {
-				fp->_p--;
-				fp->_w++;
-			}
+	if (++n == fp->_bf._size || (fp->_flags & __SLBF && c == '\n'))
+		if (__fflush(fp) != 0)
 			return (EOF);
-		}
-	}
 	return (c);
 }