From owner-svn-src-head@freebsd.org Fri Feb 26 00:50:55 2016 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 A9069AB448D; Fri, 26 Feb 2016 00:50:55 +0000 (UTC) (envelope-from truckman@FreeBSD.org) Received: from gw.catspoiler.org (unknown [IPv6:2602:304:b010:ef20::f2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "gw.catspoiler.org", Issuer "gw.catspoiler.org" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 8CA79E62; Fri, 26 Feb 2016 00:50:55 +0000 (UTC) (envelope-from truckman@FreeBSD.org) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.15.2/8.15.2) with ESMTP id u1Q0og8T010950; Thu, 25 Feb 2016 16:50:46 -0800 (PST) (envelope-from truckman@FreeBSD.org) Message-Id: <201602260050.u1Q0og8T010950@gw.catspoiler.org> Date: Thu, 25 Feb 2016 16:50:41 -0800 (PST) From: Don Lewis Subject: Re: svn commit: r296025 - head/sys/netpfil/pf To: kostikbel@gmail.com cc: kp@FreeBSD.org, cem@FreeBSD.org, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org In-Reply-To: <20160225100757.GA67250@kib.kiev.ua> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 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: Fri, 26 Feb 2016 00:50:55 -0000 On 25 Feb, Konstantin Belousov wrote: > On Thu, Feb 25, 2016 at 10:17:41AM +0100, Kristof Provost wrote: >> On 2016-02-24 23:47:55 (-0800), Conrad Meyer wrote: >> > On Wed, Feb 24, 2016 at 11:41 PM, Adrian Chadd wrote: >> > > .. what's capping totlen so one doesn't run out of memory? >> > >> > There was a DoS vector before (user controlled io->pfrio_size) and >> > basically the same DoS vector now (either of io->pfrio_size or >> > io->pfrio_size2). This change isn't a regression. Still, it should >> > be fixed. >> > >> It's an M_WAITOK allocation, so if the user asks for more memory than is >> available the thread will sleep. I'd assumed that if the user terminates >> the thread the sleep will wake, the allocation will fail and the ioctl() >> will return an error. > M_WAITOK allocations still panic when requested amount of KVA is > unreasonable. > > I am curious what do you mean by 'user terminating the thread'. The > sleep in malloc() is uninterruptible by signal, and user does not have > any other way to disturb the execution. > >> >> Perhaps we should do what OpenBSD do, and not allocate the temporary >> buffer at all. They copy in/out the individual entries one by one. On >> the other hand, one could still exhaust memory by inserting large >> numbers of addresses in the table. > But note that accesses to the user memory may fault, which puts whole > VM and VFS subsystems (and possibly the network as well, if user > supplied address is backed by mapped NFS file) after the locks owned > at the moment of copyin() call. Well, you could wire the user buffer like is done for sysctl buffers. That removes the KVA constraint, but it basically just moves the problem elsewhere. As I recall, we have limits on wired memory, though I believe it is less restrictive for transient requests. It's been ages since I looked at this.