Date: Sat, 10 Feb 2018 06:58:26 +0000 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 Message-ID: <bug-209475-17777-1BOce6GQrE@https.bugs.freebsd.org/bugzilla/> In-Reply-To: <bug-209475-17777@https.bugs.freebsd.org/bugzilla/> References: <bug-209475-17777@https.bugs.freebsd.org/bugzilla/>
next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209475 --- Comment #26 from fehmi noyan isi <fnoyanisi@yahoo.com> --- Created attachment 190476 --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=190476&action=edit Kernel Crash Dump (In reply to Kristof Provost from comment #25) The VM I am running the tests is 64-bit,so I do not think the panic is triggered by mallocarray(9). However, I see that the mtx_init(9) in the for loop causes the crash. I attach textdump output for your reference.... Here is the problem; if ((V_pf_keyhash = mallocarray(pf_hashsize, sizeof(struct pf_keyhash), M_PFHASH, M_NOWAIT | M_ZERO)) == NULL){ V_pf_keyhash = mallocarray(PF_HASHSIZ, sizeof(struct pf_keyhash), M_PFHASH, M_WAITOK | M_ZERO); printf(...); } if ((V_pf_idhash = mallocarray(pf_hashsize, sizeof(struct pf_idhash), M_PFHASH, M_NOWAIT | M_ZERO)) == NULL){ V_pf_idhash = mallocarray(PF_HASHSIZ, sizeof(struct pf_idhash), M_PFHASH, M_WAITOK | M_ZERO); printf(...); } pf_hashmask = pf_hashsize - 1; // pf_hashsize is 2147483648 for (i = 0, kh = V_pf_keyhash, ih = V_pf_idhash; i <= pf_hashmask; i++, kh++, ih++) { mtx_init(&kh->lock, "pf_keyhash", NULL, MTX_DEF | MTX_DUPOK); mtx_init(&ih->lock, "pf_idhash", NULL, MTX_DEF); } In the code above, V_ph_idhash and V_pf_keyhash are allocated PF_HASHSIZ * sizeof(struct pf_keyhash) and PF_HASHSIZ * sizeof(struct pf_idhash) amount of memory respectively. The for loop following the mallocarray(9) calls expects the allocated memory to be aligned with pf_hashsize variable, which is usd in the loop and set to 2147483648 in our example. On the other hand, PH_HASHSIZ is 32768. This mismatch causes the initialisation to fail. Apparently, the value of pf_hashsize needs to be set and it should be used in mallocarray(9) calls rather than PF_HASHSIZ. Although, sizeof(struct pf_keyhash) = sizeof(struct pf_idhash) = 40, we cannot guarantee that the size of structs will stay the same (please correct me if I am wrong). Given that the for loop assumes V_ph_idhash and V_pf_keyhash are allocated memory by using the same multiplier, which is pf_hashsize, I think we either * should make a test before the mallocarray() calls and set pf_hashsize accordingly (how?) * make two mallocarray(...,M_NOWAIT) calls and test return values in a single if() statement. If either or both of these pointers is NULL, we should fallback and re-allocate memory for _both_ V_ph_idhash and V_pf_keyhash by using a single pf_hashsize value. -- You are receiving this mail because: You are the assignee for the bug.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-209475-17777-1BOce6GQrE>