Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > 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. Its supposed to be this: * listcnt is set to 0 and match/target is added to list from select_ops * in case of error: match/target is left in the list, so it can be "recovered" * in case of success: listcnt/refcnt get increased, so nft_xt is not removed from lists when the expression is destroyed > So listcnt is there to catch the error case, right? listcnt is there to remove the expression from the list via ->deactivate, ->destroy is called too late (after commit phase/no mutex held). We need the listcnt to know when we remove the last instance of the expression. > 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. That would make it possible to init both listcnt and recnt to 1. We could also consider removing the entire list handling and always use a 1:1 mapping of nft_expr <-> nft_compat_ops (we don't need to "Recover" in case of error).
