From owner-freebsd-bugs@FreeBSD.ORG Wed Dec 7 16:10:14 2011 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7081F1065670 for ; Wed, 7 Dec 2011 16:10:14 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 46FA78FC0C for ; Wed, 7 Dec 2011 16:10:14 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id pB7GAEpf054588 for ; Wed, 7 Dec 2011 16:10:14 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id pB7GAEHC054587; Wed, 7 Dec 2011 16:10:14 GMT (envelope-from gnats) Date: Wed, 7 Dec 2011 16:10:14 GMT Message-Id: <201112071610.pB7GAEHC054587@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Jaakko Heinonen Cc: Subject: Re: kern/163076: It is not possible to read in chunks from linprocfs and procfs. X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Jaakko Heinonen List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Dec 2011 16:10:14 -0000 The following reply was made to PR kern/163076; it has been noted by GNATS. From: Jaakko Heinonen To: Poul-Henning Kamp Cc: Petr Salinger , bug-followup@FreeBSD.org, des@FreeBSD.org, mdf@FreeBSD.org Subject: Re: kern/163076: It is not possible to read in chunks from linprocfs and procfs. Date: Wed, 7 Dec 2011 18:08:34 +0200 Hi, On 2011-12-06, Jaakko Heinonen wrote: > On 2011-12-06, Poul-Henning Kamp wrote: > > >Shouldn't sbuf_finish() then check s->s_error before appending the > > >trailing '\0' and setting the SBUF_FINISHED flag? The problem in > > >question wasn't caught earlier because sbuf_finish() happily finishes > > >the buffer even if it has an error. > > > > I belive the code is written so that there is always reserved space > > for the final '\0' > > > > sbuf_finish() should finish _any_ sbuf, and return zero only if > > the finished buffer is fully OK. > > Anyway I find it inconsistent that you can successfully call > sbuf_finish() and sbuf_data() but not for example sbuf_len() on an > errored buffer. After looking at some code using sbufs I think that the sbuf(9) API change done in r222004 is problematic. Lots of code doesn't check the return value of sbuf_finish() but they expect sbuf_len() to return the actual length regardless of the error status after calling sbuf_finish(). Since r222004 sbuf_len() may return -1 after sbuf_finish(). In user space also SBUF_AUTOEXTEND buffers are affected because malloc(3) can fail and cause the error status to be set. Could we just remove the error check from sbuf_len()? (patch below) I have Cc'd more people. sbuf(9) manual page wrongly claims that sbuf_data() will return NULL if the buffer has overflowed. %%% Index: sys/kern/subr_sbuf.c =================================================================== --- sys/kern/subr_sbuf.c (revision 228153) +++ sys/kern/subr_sbuf.c (working copy) @@ -725,8 +725,6 @@ sbuf_len(struct sbuf *s) KASSERT(s->s_drain_func == NULL, ("%s makes no sense on sbuf %p with drain", __func__, s)); - if (s->s_error != 0) - return (-1); return (s->s_len); } Index: share/man/man9/sbuf.9 =================================================================== --- share/man/man9/sbuf.9 (revision 228153) +++ share/man/man9/sbuf.9 (working copy) @@ -25,7 +25,7 @@ .\" .\" $FreeBSD$ .\" -.Dd January 25, 2011 +.Dd December 7, 2011 .Dt SBUF 9 .Os .Sh NAME @@ -462,14 +462,6 @@ function returns a non-zero value if the drain error, and zero otherwise. .Pp The -.Fn sbuf_data -and -.Fn sbuf_len -functions return -.Dv NULL -and \-1, respectively, if the buffer overflowed. -.Pp -The .Fn sbuf_copyin function returns \-1 if copying string from userland failed, and number of bytes %%% -- Jaakko