d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

An off-by-one error exists in sbuf_vprintf()'s use of SBUF_HASROOM() when an

Browse Source 
sbuf is filled to capacity by vsnprintf(), the loop exits without error, and
the sbuf is not marked as auto-extendable.

SBUF_HASROOM() evaluates true if there is room for one or more non-NULL
characters, but in the case that the sbuf was filled exactly to capacity,
SBUF_HASROOM() evaluates false. Consequently, sbuf_vprintf() incorrectly
assigns an ENOMEM error to the sbuf when in fact everything is fine, in turn
poisoning the buffer for all subsequent operations.

Correct by moving the ENOMEM assignment into the loop where it can be made
unambiguously.

As a related safety net change, explicitly check for the zero bytes drained
case in sbuf_drain() and set EDEADLK as the error. This avoids an infinite loop
in sbuf_vprintf() if a drain function were to inadvertently return a value of
zero to sbuf_drain().

Reviewed by:	cem, jtl, gallatin
MFC after:	2 weeks
Sponsored by:	Netflix, Inc.
Differential Revision:	https://reviews.freebsd.org/D8535
This commit is contained in:
lstewart 2017-08-18 02:06:28 +00:00
parent b283701459
commit 5b9d72a33b
1 changed files with 5 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
sys/kern/subr_sbuf.c
View File

@ -369,8 +369,8 @@ sbuf_drain(struct sbuf *s)
return (s->s_error = EDEADLK);
len = s->s_drain_func(s->s_drain_arg, s->s_buf,
SBUF_DODRAINTOEOR(s) ? s->s_rec_off : s->s_len);
if (len < 0) {
s->s_error = -len;
if (len <= 0) {
s->s_error = len ? -len : EDEADLK;
return (s->s_error);
}
KASSERT(len > 0 && len <= s->s_len,
@ -640,9 +640,9 @@ sbuf_vprintf(struct sbuf *s, const char *fmt, va_list ap)
break;
/* Cannot print with the current available space. */
if (s->s_drain_func != NULL && s->s_len > 0)
error = sbuf_drain(s);
else
error = sbuf_extend(s, len - SBUF_FREESPACE(s));
error = sbuf_drain(s); /* sbuf_drain() sets s_error. */
else if (sbuf_extend(s, len - SBUF_FREESPACE(s)) != 0)
s->s_error = error = ENOMEM;
} while (error == 0);
/*
@ -659,8 +659,6 @@ sbuf_vprintf(struct sbuf *s, const char *fmt, va_list ap)
s->s_len += len;
if (SBUF_ISSECTION(s))
s->s_sect_len += len;
if (!SBUF_HASROOM(s) && !SBUF_CANEXTEND(s))
s->s_error = ENOMEM;
KASSERT(s->s_len < s->s_size,
("wrote past end of sbuf (%d >= %d)", s->s_len, s->s_size));