On Mon, Feb 04, 2019 at 10:05:35PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> > On Mon, Feb 04, 2019 at 06:42:05PM +0100, Florian Westphal wrote:
> > > The patch 'netfilter: nf_tables: unbind set in rule from commit path'
> > > changed .deactivate signature, so update nft_compat accordingly.
> > >
> > > We only must make sure we unlink nft_xt from list before mutex
> > > is released, i.e. abort or commit phase is enough.
> >
> > Yes, I'll append your description and will include that this is joint
> > work.
> >
> > We still should bump xt->listcnt here, right?
> >
> > /* Re-use the existing match if it's already loaded. */
> > list_for_each_entry(nft_match, &cn->nft_match_list, head) {
> > struct xt_match *match = nft_match->ops.data;
> >
> > if (nft_match_cmp(match, mt_name, rev, family))
> > return &nft_match->ops;
> > }
> >
> > I mean, in case we find a matching in the cn->nft_match_list.
>
> No, anything we change in .select_ops() cannot be undone on error
> because nf_tables core has no expr->ops function that gets called
> when select_ops() was invoked on an expression, but init() wasn't
> called yet.
>
> Thats the one major pain problem in nft_compat and the reason for
> these silly refcount/list games we need to play...
OK. So after this chunk, this works like this:
* listcnt is set to 1 and match/target is added to the list from .select_ops
* in case of error: match/target is left in the list, so it can be "recovered".
* in case of success: match/target is removed from the list, so this
nft_xt is unique.
So listcnt is there to catch the error case, right?
We can probably add something like a flag to nft_expr_ops, eg.
NFT_EXPR_F_DYNAMIC, then call a release function for this
.release_ops from the error path to solve this problem in nft_compat.
Or just check if there's a .release_ops and call it.
Thanks.
