On Thu, 5 Oct 2023, Florian Westphal wrote: > xiaolinkui <xiaolinkui@xxxxxxx> wrote: > > From: Linkui Xiao <xiaolinkui@xxxxxxxxxx> > > > > Before destroying the ipset, take a check on sequence to ensure that the > > ip_set_test operation of this ipset has been completed. > > > > The code of set_match_v4 is protected by addend=xt_write_recseq_begin() and > > xt_write_recseq_end(addend). So we can ensure that the test operation is > > completed by reading seqcount. > > Nope, please don't do this, the xt_set can also be used from nft_compat > which doesn't use the xtables packet traversers. > > I'd rather use synchonize_rcu() once in ip_set_destroy(), that will > make sure all concurrent traversers are gone. But ip_set_destroy() can be called only when there's no reference to the set in the kernel and thus there's no ipset function whatsoever in the packet path which would access it. > That said, I still do not understand this fix, the > match / target destroy hooks are called after the table has > been completely replaced, i.e., while packets can still be in flight > no packets should be within the ipset lookup functions when > this happens, and no more packets should be able to enter them. > > AFAICS the request to delete the set will fail if its still referenced > via any rule. xt_set holds references to the sets. > > So: > 1. set have dropped all references > 2. userspace *can* delete the set > 3. we get crash because xt_set was still within a sets eval > function. > > I don't see how 3) can happen, xt table replace isn't supposed > to call the xt_set destroy functions until after table replace. > > We even release the entire x_table blob right afterwards. I'd expect the author to send patches to netfilter-devel@xxxxxxxxxxxxxxx first in order to review netfilter and ipset related patches there. Best regards, Jozsef - E-mail : kadlec@xxxxxxxxxxxxxxxxx, kadlecsik.jozsef@xxxxxxxxx PGP key : https://wigner.hu/~kadlec/pgp_public_key.txt Address : Wigner Research Centre for Physics H-1525 Budapest 114, POB. 49, Hungary
