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=3D209475 --- Comment #26 from fehmi noyan isi <fnoyanisi@yahoo.com> --- Created attachment 190476 --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=3D190476&action= =3Dedit 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 =3D mallocarray(pf_hashsize, sizeof(struct pf_keyh= ash), M_PFHASH, M_NOWAIT | M_ZERO)) =3D=3D NULL){ V_pf_keyhash =3D mallocarray(PF_HASHSIZ, sizeof(struct pf_keyha= sh), M_PFHASH, M_WAITOK | M_ZERO); printf(...); } if ((V_pf_idhash =3D mallocarray(pf_hashsize, sizeof(struct pf_idha= sh), M_PFHASH, M_NOWAIT | M_ZERO)) =3D=3D NULL){ V_pf_idhash =3D mallocarray(PF_HASHSIZ, sizeof(struct pf_idhash= ), M_PFHASH, M_WAITOK | M_ZERO); printf(...); } pf_hashmask =3D pf_hashsize - 1; // pf_hashsize is 2147483648 for (i =3D 0, kh =3D V_pf_keyhash, ih =3D V_pf_idhash; i <=3D pf_ha= shmask; 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.=20 The for loop following the mallocarray(9) calls expects the allocated memor= y 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.=20 Although, sizeof(struct pf_keyhash) =3D sizeof(struct pf_idhash) =3D 40, we= cannot guarantee that the size of structs will stay the same (please correct me if= I am wrong).=20 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 sin= gle if() statement. If either or both of these pointers is NULL, we should fall= back and re-allocate memory for _both_ V_ph_idhash and V_pf_keyhash by using a single pf_hashsize value. --=20 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>