From owner-freebsd-pf@FreeBSD.ORG Thu Sep 20 02:12:32 2012 Return-Path: Delivered-To: freebsd-pf@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1E134106566B; Thu, 20 Sep 2012 02:12:32 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebius.int.ru (glebius.int.ru [81.19.64.117]) by mx1.freebsd.org (Postfix) with ESMTP id 6915E8FC19; Thu, 20 Sep 2012 02:12:31 +0000 (UTC) Received: from cell.glebius.int.ru (localhost [127.0.0.1]) by cell.glebius.int.ru (8.14.5/8.14.5) with ESMTP id q8K2CTp9044001; Thu, 20 Sep 2012 06:12:29 +0400 (MSK) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebius.int.ru (8.14.5/8.14.5/Submit) id q8K2CTwD044000; Thu, 20 Sep 2012 06:12:29 +0400 (MSK) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebius.int.ru: glebius set sender to glebius@FreeBSD.org using -f Date: Thu, 20 Sep 2012 06:12:29 +0400 From: Gleb Smirnoff To: Ermal Lu?i Message-ID: <20120920021229.GN85604@glebius.int.ru> References: <201209181234.q8ICYaFB091109@svn.freebsd.org> <20120918161516.GG85604@glebius.int.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=koi8-r Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Sergey Kandaurov , freebsd-pf@FreeBSD.org Subject: Re: svn commit: r240646 - head/sys/contrib/altq/altq X-BeenThere: freebsd-pf@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Technical discussion and general questions about packet filter \(pf\)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Sep 2012 02:12:32 -0000 Ermal, On Wed, Sep 19, 2012 at 09:42:47PM +0200, Ermal Lu?i wrote: E> > On Tue, Sep 18, 2012 at 06:02:06PM +0200, Ermal Lu?i wrote: E> > E> The issue is that this hides the problem per se. E> > E> > What had hidden problem per se, was the following code: E> > E> > PF_UNLOCK(); E> > error = altq_add(a2); E> > PF_LOCK(); E> > E> > That's what we have in stable/9. E> > E> > E> The ioctl and pfctl loading of ruleset is not ready for handling failures here! E> > E> > They do. Error from altq_add() is returned by pf_ioctl() as response E> > to DIOCADDALTQ command. The code in pfctl, which does DIOCADDALTQ also E> > is handling errors. E> E> The issue is that you will fail a ruleset loading now that before E> could not fail. Before you could just race with some other thread modifing ALTQ, for example ifnet departure/attachment event, which would lead to panic. E> You need to teach pfctl that is ok if ALTQ ruleset load fails now, no? E> E> I think the most important thing in ruleset loading is the rules than E> comes ALTQ. E> Since ALTQ failure is tolerable and the risk from that faling is low! E> Its better to do a best effort loading of ruleset E> and just report where it failed? Configuring rulesets also does a lot of malloc(9) that can fail. Thus, if ALTQ configuration failed due to malloc(9) failure, then probably ruleset loading would fail, too. Usually, if system is low on kernel memory, most things won't work at all. So this isn't a case that should be optimized right now. If ruleset configuration would be refactored to a state when first all malloc()s are issued with M_WAITOK flag and only then rules lock is acquired, then we can get back to achieving same functionality for ALTQ. E> You just committed a 'questionable' patch for default block, just for E> security, though E> break that contract by making security depend on unpredictable behaviour! If you ponder this a second longer then you'll see, that default to block policy actually makes security depend less on unpredictable behavior: if anything goes wrong during boot, then a box is closed. -- Totus tuus, Glebius.