Re: [Patch nf] netfilter: xt_hashlimit: unregister proc file before releasing mutex

On Tue, Feb 18, 2020 at 2:05 PM Pablo Neira Ayuso [off-list ref] wrote:

On Tue, Feb 18, 2020 at 01:40:26PM -0800, Cong Wang wrote:

quoted

On Tue, Feb 18, 2020 at 1:35 PM Pablo Neira Ayuso [off-list ref] wrote:

quoted

On Wed, Feb 12, 2020 at 10:53:52PM -0800, Cong Wang wrote:

quoted

Before releasing the global mutex, we only unlink the hashtable
from the hash list, its proc file is still not unregistered at
this point. So syzbot could trigger a race condition where a
parallel htable_create() could register the same file immediately
after the mutex is released.

Move htable_remove_proc_entry() back to mutex protection to
fix this. And, fold htable_destroy() into htable_put() to make
the code slightly easier to understand.

Probably revert previous one?

The hung task could appear again if we move the cleanup
back under mutex.

How could the hung task appear again by reverting
c4a3922d2d20c710f827? Please elaborate.

Because the cfg.max could be as large as 8*HASHLIMIT_MAX_SIZE:

 311         if (hinfo->cfg.max == 0)
 312                 hinfo->cfg.max = 8 * hinfo->cfg.size;
 313         else if (hinfo->cfg.max < hinfo->cfg.size)
 314                 hinfo->cfg.max = hinfo->cfg.size;

Not sure whether we can finish cleaning up 8*HASHLIMIT_MAX_SIZE
entries within the time a hung task tolerates. This largely depends on
how much contention the spinlock has, at least I don't want to bet
on it.

Thanks.