svn commit: r322643 - head/sys/kern

[Lawrence Stewart]

Author: lstewart
Date: Fri Aug 18 02:06:28 2017
New Revision: 322643
URL: https://svnweb.freebsd.org/changeset/base/322643

Log:
  An off-by-one error exists in sbuf_vprintf()'s use of SBUF_HASROOM() when an
  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

Modified:
  head/sys/kern/subr_sbuf.c

Modified: head/sys/kern/subr_sbuf.c
==============================================================================
--- head/sys/kern/subr_sbuf.c	Fri Aug 18 01:34:38 2017	(r322642)
+++ head/sys/kern/subr_sbuf.c	Fri Aug 18 02:06:28 2017	(r322643)
@@ -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 
 			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 
 	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));