From owner-svn-src-head@freebsd.org Sat Feb 25 07:07:28 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 574FBCECE43; Sat, 25 Feb 2017 07:07:28 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebi.us (glebi.us [96.95.210.25]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "cell.glebi.us", Issuer "cell.glebi.us" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 37823891; Sat, 25 Feb 2017 07:07:27 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebi.us (localhost [127.0.0.1]) by cell.glebi.us (8.15.2/8.15.2) with ESMTPS id v1P77QxF029689 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 24 Feb 2017 23:07:26 -0800 (PST) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebi.us (8.15.2/8.15.2/Submit) id v1P77PIW029688; Fri, 24 Feb 2017 23:07:25 -0800 (PST) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebi.us: glebius set sender to glebius@FreeBSD.org using -f Date: Fri, 24 Feb 2017 23:07:25 -0800 From: Gleb Smirnoff To: hiren panchasara Cc: "Simon J. Gerraty" , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r278729 - head/sys/sys Message-ID: <20170225070725.GC8899@FreeBSD.org> References: <201502132319.t1DNJZuP057045@svn.freebsd.org> <20150311213607.GN88380@strugglingcoder.info> <20150316123940.GA17947@FreeBSD.org> <20150317010654.GB53237@strugglingcoder.info> <20150319180812.GI53237@strugglingcoder.info> <20170217190621.GE66077@strugglingcoder.info> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170217190621.GE66077@strugglingcoder.info> User-Agent: Mutt/1.7.2 (2016-11-26) 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: Sat, 25 Feb 2017 07:07:28 -0000 On Fri, Feb 17, 2017 at 11:06:21AM -0800, hiren panchasara wrote: h> On 03/19/15 at 11:08P, hiren panchasara wrote: h> > On 03/16/15 at 06:06P, hiren panchasara wrote: h> > > On 03/16/15 at 03:39P, Gleb Smirnoff wrote: h> > > > On Wed, Mar 11, 2015 at 02:36:07PM -0700, hiren panchasara wrote: h> > > > h> On 02/13/15 at 11:19P, Simon J. Gerraty wrote: h> > > > h> > Author: sjg h> > > > h> > Date: Fri Feb 13 23:19:35 2015 h> > > > h> > New Revision: 278729 h> > > > h> > URL: https://svnweb.freebsd.org/changeset/base/278729 h> > > > h> > h> > > > h> > Log: h> > > > h> > sbspace: size of bleft, mleft must match sockbuf fields to avoid h> > > > h> > overflow on amd64 h> > > > h> > h> > > > h> > Submitted by: anshukla@juniper.net h> > > > h> > Obtained from: Juniper Networks h> > > > h> h> > > > h> Talking to sjg on -arch to MFC this. If he cannot get around doing that, h> > > > h> I'll do it tomorrow. h> > > > h> h> > > > h> Letting people know here to see if there are any objections. h> > > > h> > > > Would that fix the bug we've been discussing? h> > > h> > > Unsure as I am not sure what caused the issue I saw. h> > > h> > > For those who do not know the details, we recently saw a userland h> > > process stuck spinning at 100% around sbcut_internal(). Inside h> > > sbflush_internal(), the sb_cc was grown to be about 4G. And before h> > > passing it to sbcut_internal(), we cast it from uint to int which h> > > would make that valud -ve. h> > > h> > > Gleb pointed out to me that sbspace() is supposed to check/stop sb_cc h> > > from growing that large. h> > > h> > > Now, I am not sure if we'd ever run into this situation again but h> > > current fix is a great catch anyways. h> > > h> > > I still have 2 questions around what we saw. It'd be great if someone can h> > > clarify them for my understanding: h> > > h> > > 1) Even if we get into such a scenario that we were in, following would h> > > help by not looping endlessly. h> > > h> > > --- uipc_sockbuf.c.0 2015-03-11 15:49:52.000000000 -0700 h> > > +++ uipc_sockbuf.c 2015-03-11 15:51:48.000000000 -0700 h> > > @@ -877,6 +877,9 @@ h> > > { h> > > struct mbuf *m, *n, *next, *mfree; h> > > h> > > + if (len < 0) h> > > + panic("%s: len is %d and it is supposed to be +ve", h> > > + __func__, len); h> > > + h> > > next = (m = sb->sb_mb) ? m->m_nextpkt : 0; h> > > mfree = NULL h> > > h> > > 2) We need 1) because we are casting a uint to int which _may_ rander a h> > > value -ve. Is there a way we can avoid the casting? h> > h> > It'd be useful if someone with knowledge in this area can weigh in. h> h> Ran into this again today. While the real question of how sb_ccc grew h> this large is still unsolved, any objection to adding this patch to h> avoid a hang and panic instead? I am all for adding KASSERT now and then fixing the bug. -- Totus tuus, Glebius.