From owner-svn-src-head@freebsd.org Tue Sep 3 14:07:20 2019 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id EE2E4DD397; Tue, 3 Sep 2019 14:07:02 +0000 (UTC) (envelope-from yuripv@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [96.47.72.132]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "freefall.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46N8063v9tz4QBH; Tue, 3 Sep 2019 14:07:02 +0000 (UTC) (envelope-from yuripv@freebsd.org) Received: by freefall.freebsd.org (Postfix, from userid 1452) id A4F831AFCD; Tue, 3 Sep 2019 14:06:26 +0000 (UTC) X-Original-To: yuripv@localmail.freebsd.org Delivered-To: yuripv@localmail.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client CN "mx1.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by freefall.freebsd.org (Postfix) with ESMTPS id A405B19F04; Wed, 17 Apr 2019 22:05:36 +0000 (UTC) (envelope-from owner-src-committers@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [96.47.72.132]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "freefall.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 40F9895BED; Wed, 17 Apr 2019 22:05:36 +0000 (UTC) (envelope-from owner-src-committers@freebsd.org) Received: by freefall.freebsd.org (Postfix, from userid 538) id 10F6F19EF1; Wed, 17 Apr 2019 22:05:36 +0000 (UTC) Delivered-To: src-committers@localmail.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [96.47.72.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client CN "mx1.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by freefall.freebsd.org (Postfix) with ESMTPS id 118AB19EEE; Wed, 17 Apr 2019 22:05:33 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id D22D295BE8; Wed, 17 Apr 2019 22:05:32 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from venus.codepro.be (venus.codepro.be [5.9.86.228]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "smtp.codepro.be", Issuer "Let's Encrypt Authority X3" (verified OK)) (Authenticated sender: kp) by smtp.freebsd.org (Postfix) with ESMTPSA id 6C28F10F0; Wed, 17 Apr 2019 22:05:32 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from [192.168.228.1] (host1.test.ravensoft.telecomplete.net [213.160.118.146]) (Authenticated sender: kp) by venus.codepro.be (Postfix) with ESMTPSA id 4E0E51A8BE; Thu, 18 Apr 2019 00:05:29 +0200 (CEST) From: "Kristof Provost" To: "Gleb Smirnoff" Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r346319 - head/sys/netpfil/pf X-Mailer: MailMate (2.0BETAr6135) Message-ID: <8B606261-B614-474F-AA73-217FE09E28BA@FreeBSD.org> In-Reply-To: <20190417211718.GX1243@FreeBSD.org> References: <201904171642.x3HGgslA075253@repo.freebsd.org> <20190417211718.GX1243@FreeBSD.org> MIME-Version: 1.0 Precedence: bulk X-Loop: FreeBSD.org Sender: owner-src-committers@freebsd.org X-Rspamd-Queue-Id: 40F9895BED X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.98 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.98)[-0.975,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] Status: O Content-Type: text/plain; charset=utf-8; format=flowed; markup=markdown Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Date: Tue, 03 Sep 2019 14:07:21 -0000 X-Original-Date: Wed, 17 Apr 2019 23:05:26 +0100 X-List-Received-Date: Tue, 03 Sep 2019 14:07:21 -0000 On 17 Apr 2019, at 22:17, Gleb Smirnoff wrote: > Kristof, > > On Wed, Apr 17, 2019 at 04:42:54PM +0000, Kristof Provost wrote: > K> Modified: head/sys/netpfil/pf/pf_ioctl.c > K> > ============================================================================== > K> --- head/sys/netpfil/pf/pf_ioctl.c Wed Apr 17 16:31:30 > 2019 (r346318) > K> +++ head/sys/netpfil/pf/pf_ioctl.c Wed Apr 17 16:42:54 > 2019 (r346319) > K> @@ -3103,24 +3103,24 @@ DIOCCHANGEADDR_error: > K> break; > K> } > K> > K> - PF_RULES_WLOCK(); > K> + PF_RULES_RLOCK(); > K> n = pfr_table_count(&io->pfrio_table, io->pfrio_flags); > K> io->pfrio_size = min(io->pfrio_size, n); > K> + PF_RULES_RUNLOCK(); > K> > K> totlen = io->pfrio_size * sizeof(struct pfr_table); > K> pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table), > K> M_TEMP, M_NOWAIT); > K> if (pfrts == NULL) { > K> error = ENOMEM; > K> - PF_RULES_WUNLOCK(); > K> break; > K> } > K> error = copyin(io->pfrio_buffer, pfrts, totlen); > K> if (error) { > K> free(pfrts, M_TEMP); > K> - PF_RULES_WUNLOCK(); > K> break; > K> } > K> + PF_RULES_WLOCK(); > K> error = pfr_set_tflags(pfrts, io->pfrio_size, > K> io->pfrio_setflag, io->pfrio_clrflag, &io->pfrio_nchange, > K> &io->pfrio_ndel, io->pfrio_flags | PFR_FLAG_USERIOCTL); > > Couple comments: > > 1) Now we can malloc with M_WAITOK. > That’s a good point. I’ll see about changing that tomorrow. > 2) Are we sure that table count won't change while we dropped the > lock? > No, the table count can indeed change while we’re unlocked. It doesn’t really matter though. The initial count only serves to limit the memory allocation to something sane. pfr_set_tflags() still does appropriate checks. It’s always been possible for the table count to change between user space preparing its request and it being handled in the kernel, so that was always a possibility. Regards, Kristof