From owner-svn-src-head@freebsd.org Fri Aug 18 02:06:30 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 0038CDE673C; Fri, 18 Aug 2017 02:06:30 +0000 (UTC) (envelope-from lstewart@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C0C0F6E0DC; Fri, 18 Aug 2017 02:06:29 +0000 (UTC) (envelope-from lstewart@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v7I26SNC061402; Fri, 18 Aug 2017 02:06:28 GMT (envelope-from lstewart@FreeBSD.org) Received: (from lstewart@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v7I26SaA061401; Fri, 18 Aug 2017 02:06:28 GMT (envelope-from lstewart@FreeBSD.org) Message-Id: <201708180206.v7I26SaA061401@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: lstewart set sender to lstewart@FreeBSD.org using -f From: Lawrence Stewart Date: Fri, 18 Aug 2017 02:06:28 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r322643 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: lstewart X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 322643 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Aug 2017 02:06:30 -0000 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));