Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 16 Jan 2018 22:20:11 +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-HlK8VbhtBO@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 #16 from fehmi noyan isi <fnoyanisi@yahoo.com> ---
(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.=



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-209475-17777-HlK8VbhtBO>