On Wed, 4 Jul 2018 18:54:55 +0200 Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > On Tue, Jul 03, 2018 at 02:05:12PM +0200, Stefano Brivio wrote: > > Hi, > > > > On Mon, 2 Jul 2018 19:56:37 +0200 (CEST) > > Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx> wrote: > > > > > > As a result, when sets are listed, ip_set_name_byindex() might > > > > now fetch a set whose reference count is already zero. Instead > > > > of relying on the reference count to protect against concurrent > > > > set renaming and listing, note that those two operations are > > > > serialised by the nfnl mutex, and that the set itself is > > > > protected by RCU nowadays. > > > > > > Listing is not serialized by the nfnl mutex because a netlink dump is used > > > behind it. So I believe the patch is not correct and therefore I cannot > > > apply it. > > > > Thanks for checking. But my understanding is that the list operation > > still goes through nfnetlink, and nfnetlink_rcv_msg() takes the nfnl > > mutex. > > > > I tested this assumption by checking that ip_set_name_byindex() and > > ip_set_rename() don't run concurrently if I run concurrent loops of > > 'ipset list' and 'ipset rename', and also by adding a: > > > > WARN_ON(subsys_id == NFNL_SUBSYS_IPSET); > > > > in nfnl_lock() and nfnl_unlock(), and it triggers on 'ipset list'. What > > am I missing? > > Only the netlink_recvmsg() first call is protected under nfnl lock, > follow up calls happen from the netlink_dump() path which in netfilter > is rcu based. Of course, I see now, thanks for the explanation! > We have callbacks in nfnetlink to achieve full rcu dumps. So I guess you are suggesting to change ipset dumps to use those callbacks. Unless Jozsef has any objection, I would, at least for the moment being, fix this specific issue in another way (i.e. using the ref_netlink refcount as it's already done for swap and delete operations), and then at some point consider again this idea. -- Stefano -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
