Date: Wed, 7 Dec 2011 16:10:14 GMT From: Jaakko Heinonen <jh@FreeBSD.org> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/163076: It is not possible to read in chunks from linprocfs and procfs. Message-ID: <201112071610.pB7GAEHC054587@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/163076; it has been noted by GNATS.
From: Jaakko Heinonen <jh@FreeBSD.org>
To: Poul-Henning Kamp <phk@phk.freebsd.dk>
Cc: Petr Salinger <Petr.Salinger@seznam.cz>, 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201112071610.pB7GAEHC054587>