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. We have callbacks in nfnetlink to achieve full rcu dumps. -- 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
