From nobody Tue Nov 07 13:37:31 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4SPq5l33gvz506J1; Tue, 7 Nov 2023 13:37:31 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4SPq5l2LZSz4RpK; Tue, 7 Nov 2023 13:37:31 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1699364251; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=E3yPGDKXw2rPM3Jltxt8JaFa8z0DP5cmuycCdxnupzA=; b=qf9as/JNnwS++S26Ho6vXXJWsgZEOtG+Cq3W+W4i6SEjMnslLh4LB8/p1TGVrlM4YjgWAm jU82BMo+ooaU6hzOmg+nOS3aVU4NC7B77F90MEEykle3kO+8zFMybjnChD/atsglb9SjUs JLFseq91egySWebmu9BqbTlUbCJpeGU5ezep69dYrx4s86A0JzBGkvWlLdaTSIMj4lp6lk b+w5GKPzQPKgMy7C5nx4WUvTG1mXUiuWknN3oya7xomXIVVICFP6QwwYEiM12kMq8XHu9g RzQOhGdII/GaqvgTdcGh0/H9PJ/DYC5Lfx1K0qa2+9NIX1zk+GAbe/lxmTy1Gw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1699364251; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=E3yPGDKXw2rPM3Jltxt8JaFa8z0DP5cmuycCdxnupzA=; b=pYmOdHLyFENc/fryJ5GuM7oEluHfV6vLtXKdFoOf0O0yrvg6dXLeXrVyAj+UU0gWwWlvvT S0ceF2orRCQJ2E6eLn4sYfBx0ABjpTQ9HVpuRlxWGHuw6zEVlpQfqT4lYrNE/lDRuoQYJX oPF+TCWGPadXN6D/w72hUR+51PGwkT4BBcfFi774vKNQi5s+Y88+U2RBysbtum/h8THaf0 mZ0mW0AMf3oPhrgv9YSAF1afcviksNIZMqdMHN5EUvdGlAyImwXuxuevR2WzmMpzCiz273 rcKVRNcaoqVWQCmH8PW/RaV//9fflWPAOXuT1FR/SRbKo2EubLLKrJ9y2w1Log== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1699364251; a=rsa-sha256; cv=none; b=vzpScicZE7k7kQKAWOnC140SSY6QMpOVZI5n/rgg9c7eyeZX7OkxbMSNhP6G1DjVzvcvLb dg7/CeWPlSvppiRQebTh+0ne0jltfoH0fjFVyRYgTDXuT0BL/pxeajvxuZ7lV7ycvYawld roLSKlWMhe1hz7N9mHc5W5988k07J6VWasOR/P2E5Q4wWGGSg/01P72iooU+JvZ0zCtzpT GjSKtWSJn27temJ7R6+4oouFuzYHmenyw6BE/qNgbPpajlSPEIR4iX2ti/Vzyt3e7HrJvq pTEAdHnRKbLH0pZiqr9w6oltjbIK6SagWKVAB/Vxa7JYypRowjavoBFam1dTKA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4SPq5l1P0Pzs3m; Tue, 7 Nov 2023 13:37:31 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 3A7DbV5l051807; Tue, 7 Nov 2023 13:37:31 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 3A7DbVlm051804; Tue, 7 Nov 2023 13:37:31 GMT (envelope-from git) Date: Tue, 7 Nov 2023 13:37:31 GMT Message-Id: <202311071337.3A7DbVlm051804@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Ed Maste Subject: git: d09a3bf72c0b - main - fflush: correct buffer handling in __sflush List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: emaste X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: d09a3bf72c0b5f1779c52269671872368c99f02a Auto-Submitted: auto-generated The branch main has been updated by emaste: URL: https://cgit.FreeBSD.org/src/commit/?id=d09a3bf72c0b5f1779c52269671872368c99f02a commit d09a3bf72c0b5f1779c52269671872368c99f02a Author: Dag-Erling Smørgrav AuthorDate: 2023-08-03 15:13:45 +0000 Commit: Ed Maste CommitDate: 2023-11-07 13:21:12 +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. --- lib/libc/stdio/fflush.c | 21 +++++++-------------- lib/libc/stdio/fvwrite.c | 4 ++-- lib/libc/stdio/wbuf.c | 2 +- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/lib/libc/stdio/fflush.c b/lib/libc/stdio/fflush.c index c658fbec697d..af2164ab3be2 100644 --- a/lib/libc/stdio/fflush.c +++ b/lib/libc/stdio/fflush.c @@ -102,8 +102,8 @@ __weak_reference(__fflush, fflush_unlocked); int __sflush(FILE *fp) { - unsigned char *p, *old_p; - int n, f, t, old_w; + unsigned char *p; + int n, f, t; f = fp->_flags; if ((f & __SWR) == 0) @@ -118,26 +118,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 = 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 3bb2f3fb6d9c..301e0d6f5e58 100644 --- a/lib/libc/stdio/fvwrite.c +++ b/lib/libc/stdio/fvwrite.c @@ -138,7 +138,7 @@ __sfvwrite(FILE *fp, struct __suio *uio) fp->_p += w; old_p = fp->_p; if (__fflush(fp) == EOF) { - if (old_p == fp->_p && errno == EINTR) + if (old_p == fp->_p) fp->_p -= w; goto err; } @@ -182,7 +182,7 @@ __sfvwrite(FILE *fp, struct __suio *uio) fp->_p += w; old_p = fp->_p; if (__fflush(fp) == EOF) { - if (old_p == fp->_p && errno == EINTR) + if (old_p == fp->_p) fp->_p -= w; goto err; } diff --git a/lib/libc/stdio/wbuf.c b/lib/libc/stdio/wbuf.c index 27ec67d28d80..7153350c99f1 100644 --- a/lib/libc/stdio/wbuf.c +++ b/lib/libc/stdio/wbuf.c @@ -88,7 +88,7 @@ __swbuf(int c, FILE *fp) 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) { + if (fp->_p == old_p) { fp->_p--; fp->_w++; }