mirror of
https://github.com/freebsd/freebsd-src
synced 2024-10-03 15:15:01 +00:00
fflush: correct buffer handling in __sflush
This fixes CVE-2014-8611 correctly. The commit that purported to fix CVE-2014-8611 (805288c2f0
) only hid it behind another bug. Two later commits,86a16ada1e
and44cf1e5eb4
, 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:805288c2f0
Fixes:86a16ada1e
Fixes:44cf1e5eb4
Sponsored by: Klara, Inc. (cherry picked from commitd09a3bf72c
) Approved by: so
This commit is contained in:
parent
51224d6d2e
commit
92709431b1
|
@ -103,8 +103,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)
|
||||
|
@ -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 = 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);
|
||||
}
|
||||
|
|
|
@ -139,7 +139,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;
|
||||
}
|
||||
|
@ -183,7 +183,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;
|
||||
}
|
||||
|
|
|
@ -89,7 +89,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++;
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue