On 03/16/15 at 03:39P, Gleb Smirnoff wrote: > On Wed, Mar 11, 2015 at 02:36:07PM -0700, hiren panchasara wrote: > h> On 02/13/15 at 11:19P, Simon J. Gerraty wrote: > h> > Author: sjg > h> > Date: Fri Feb 13 23:19:35 2015 > h> > New Revision: 278729 > h> > URL: https://svnweb.freebsd.org/changeset/base/278729 > h> > > h> > Log: > h> > sbspace: size of bleft, mleft must match sockbuf fields to avoid > h> > overflow on amd64 > h> > > h> > Submitted by: anshu...@juniper.net > h> > Obtained from: Juniper Networks > h> > h> Talking to sjg on -arch to MFC this. If he cannot get around doing that, > h> I'll do it tomorrow. > h> > h> Letting people know here to see if there are any objections. > > Would that fix the bug we've been discussing?
Unsure as I am not sure what caused the issue I saw.
For those who do not know the details, we recently saw a userland
process stuck spinning at 100% around sbcut_internal(). Inside
sbflush_internal(), the sb_cc was grown to be about 4G. And before
passing it to sbcut_internal(), we cast it from uint to int which
would make that valud -ve.
Gleb pointed out to me that sbspace() is supposed to check/stop sb_cc
from growing that large.
Now, I am not sure if we'd ever run into this situation again but
current fix is a great catch anyways.
I still have 2 questions around what we saw. It'd be great if someone can
clarify them for my understanding:
1) Even if we get into such a scenario that we were in, following would
help by not looping endlessly.
--- uipc_sockbuf.c.0 2015-03-11 15:49:52.000000000 -0700
+++ uipc_sockbuf.c 2015-03-11 15:51:48.000000000 -0700
@@ -877,6 +877,9 @@
{
struct mbuf *m, *n, *next, *mfree;
+ if (len < 0)
+ panic("%s: len is %d and it is supposed to be +ve",
+ __func__, len);
+
next = (m = sb->sb_mb) ? m->m_nextpkt : 0;
mfree = NULL
2) We need 1) because we are casting a uint to int which _may_ rander a
value -ve. Is there a way we can avoid the casting?
cheers,
Hiren
pgpQ0hJEmtm_E.pgp
Description: PGP signature
