From owner-freebsd-pf@freebsd.org Tue Jan 16 22:20:11 2018 Return-Path: Delivered-To: freebsd-pf@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 EBDAAEBC205 for ; Tue, 16 Jan 2018 22:20:11 +0000 (UTC) (envelope-from bugzilla-noreply@freebsd.org) Received: from mxrelay.ysv.freebsd.org (mxrelay.ysv.freebsd.org [IPv6:2001:1900:2254:206a::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.ysv.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id D2CEC700C1 for ; Tue, 16 Jan 2018 22:20:11 +0000 (UTC) (envelope-from bugzilla-noreply@freebsd.org) Received: from kenobi.freebsd.org (kenobi.freebsd.org [IPv6:2001:1900:2254:206a::16:76]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.ysv.freebsd.org (Postfix) with ESMTPS id C848B5A8D for ; Tue, 16 Jan 2018 22:20:11 +0000 (UTC) (envelope-from bugzilla-noreply@freebsd.org) Received: from kenobi.freebsd.org ([127.0.1.118]) by kenobi.freebsd.org (8.15.2/8.15.2) with ESMTP id w0GMKBQh027236 for ; Tue, 16 Jan 2018 22:20:11 GMT (envelope-from bugzilla-noreply@freebsd.org) Received: (from www@localhost) by kenobi.freebsd.org (8.15.2/8.15.2/Submit) id w0GMKB7Y027235 for freebsd-pf@FreeBSD.org; Tue, 16 Jan 2018 22:20:11 GMT (envelope-from bugzilla-noreply@freebsd.org) X-Authentication-Warning: kenobi.freebsd.org: www set sender to bugzilla-noreply@freebsd.org using -f From: bugzilla-noreply@freebsd.org To: freebsd-pf@FreeBSD.org Subject: [Bug 209475] pf didn't check if enough free RAM for net.pf.states_hashsize Date: Tue, 16 Jan 2018 22:20:11 +0000 X-Bugzilla-Reason: AssignedTo X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: Base System X-Bugzilla-Component: kern X-Bugzilla-Version: 10.3-RELEASE X-Bugzilla-Keywords: X-Bugzilla-Severity: Affects Only Me X-Bugzilla-Who: fnoyanisi@yahoo.com X-Bugzilla-Status: New X-Bugzilla-Resolution: X-Bugzilla-Priority: --- X-Bugzilla-Assigned-To: freebsd-pf@FreeBSD.org X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: https://bugs.freebsd.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-BeenThere: freebsd-pf@freebsd.org X-Mailman-Version: 2.1.25 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: Tue, 16 Jan 2018 22:20:12 -0000 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D209475 --- Comment #16 from fehmi noyan isi --- (In reply to Kristof Provost from comment #15) >> It does now. It was added very recently (in head). man 9 mallocarray. It= might be worth doing that change in a separate commit. This is news to me! A quick look at the head revealed quite a few commits t= hat replaces malloc(9) with mallocarray(9). That's good... However, I would say even the first problem you mentioned needs to be split into two as we can have cases where the requested amount of memory does not overflow but exceeds the available memory. Attempting malloc() (or using mallocarray() in this situation would yield t= he same result) and checking return value, then in case NULL is returned, re-attempting another allocation does not sound like the best approach that could possibly used here. How about the two-step process below? 1 ) making sure the requested allocation size does not exceed the system me= mory or the available free memory- we need to check this 2 ) avoiding overflow - mallocarray(9) would do this which can be represented in pseudo-code as given below pf_initiazlize(){ /* the first check */ alloc_size =3D check_if_we_have_enough_memory(pf_hashsize * sizeof(pf_id= hash)) /* it should be safe at this point, but better to be cautious than sad */ V_pf_srchash =3D mallocarray(alloc_size, ... ,M_NOWAIT|M_ZERO) } size_t check_if_we_have_enough_memory(u_long size) { return (size > available_or_free_memory)? PF_HASHSIZ : size; } I think the second issue below can be addressed independently from the one I mentioned above. >This suggests that there's a race in pf initialisation where the ioctl() h= andler is already registered before all initialisation is done (i.e. the la= rge allocation is still blocked). That's a bug too, and one that could pote= ntially occur even if the large allocation succeeds (though in a very small= time window). --=20 You are receiving this mail because: You are the assignee for the bug.=