The following reply was made to PR kern/163076; it has been noted by GNATS.
From: Jaakko Heinonen <[email protected]> To: Poul-Henning Kamp <[email protected]> Cc: Petr Salinger <[email protected]>, [email protected], [email protected], [email protected] 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 _______________________________________________ [email protected] mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-bugs To unsubscribe, send any mail to "[email protected]"
