netdev - Re: [PATCH net] inet: frags: rework rhashtable dismantle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 2 Oct 2018 08:58:41 +0800
From:   Herbert Xu <herbert@...dor.apana.org.au>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Thomas Graf <tgraf@...g.ch>
Subject: Re: [PATCH net] inet: frags: rework rhashtable dismantle

On Mon, Oct 01, 2018 at 10:58:21AM -0700, Eric Dumazet wrote:
>
>  void inet_frags_exit_net(struct netns_frags *nf)
>  {
> +	struct rhashtable_iter hti;
> +	struct inet_frag_queue *fq;
> +
> +	/* Since we want to cleanup the hashtable, make sure that
> +	 * we wont trigger an automatic shrinking while in our
> +	 * rhashtable_walk_next() loop.
> +	 * Also make sure that no resize is in progress.
> +	 */
>  	nf->high_thresh = 0; /* prevent creation of new frags */
> +	nf->rhashtable.p.automatic_shrinking = false;
> +	cancel_work_sync(&nf->rhashtable.run_work);
>  
> -	rhashtable_free_and_destroy(&nf->rhashtable, inet_frags_free_cb, NULL);
> +	rhashtable_walk_enter(&nf->rhashtable, &hti);
> +	rhashtable_walk_start(&hti);
> +	while ((fq = rhashtable_walk_next(&hti)) != NULL) {
> +		if (IS_ERR(fq)) /* should not happen */
> +			break;
> +		if (!del_timer_sync(&fq->timer))
> +			continue;
> +
> +		spin_lock_bh(&fq->lock);
> +		inet_frag_kill(fq);
> +		spin_unlock_bh(&fq->lock);
> +
> +		inet_frag_put(fq);
> +		if (need_resched()) {
> +			rhashtable_walk_stop(&hti);
> +			cond_resched();
> +			rhashtable_walk_start(&hti);
> +		}
> +	}

The walk interface was designed to handle read-only iteration
through the hash table.  While this probably works since the
actual freeing is delayed by RCU, it seems to be rather fragile.

How about using the dead flag but instead of putting it in the
rhashtable put it in netns_frags and have the timers check on that
before calling rhashtable_remove?

Cheers,
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt