Re: [PATCH v2 nf-next 2/2] netfilter: nf_tables: support updating userdata for nft_table

[Quan Tian <tianquan23@gmail.com>]

Hi Florian,

Thanks a lot for your reviews.

On Sun, Mar 10, 2024 at 06:47:54PM +0100, Florian Westphal wrote:
> Quan Tian <tianquan23@gmail.com> wrote:
> > @@ -10129,14 +10154,12 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
> >  		switch (trans->msg_type) {
> >  		case NFT_MSG_NEWTABLE:
> >  			if (nft_trans_table_update(trans)) {
> > -				if (!(trans->ctx.table->flags & __NFT_TABLE_F_UPDATE)) {
> > -					nft_trans_destroy(trans);
> > -					break;
> > +				if (trans->ctx.table->flags & __NFT_TABLE_F_UPDATE) {
> > +					if (trans->ctx.table->flags & NFT_TABLE_F_DORMANT)
> > +						nf_tables_table_disable(net, trans->ctx.table);
> > +					trans->ctx.table->flags &= ~__NFT_TABLE_F_UPDATE;
> >  				}
> > -				if (trans->ctx.table->flags & NFT_TABLE_F_DORMANT)
> > -					nf_tables_table_disable(net, trans->ctx.table);
> > -
> > -				trans->ctx.table->flags &= ~__NFT_TABLE_F_UPDATE;
> > +				swap(trans->ctx.table->udata, nft_trans_table_udata(trans));
> >  			} else {
> >  				nft_clear(net, trans->ctx.table);
> >  			}
> 
> There is a call to nft_trans_destroy() below here.
> You could add a "break" after the swap() to avoid it.
> 
> Otherwise kmemleak should report old udata being lost
> on update.

Thanks for pointing it out. kmemleak indeed reported the leak.
I found "break" after swap() would skip sending the table update event,
so I changed to execute different code paths for the two branches in v3.
Please let me know if it makes sense to you.

Thanks,
Quan