On Sun, Sep 10, 2017 at 4:45 AM, Xin Long <lucien....@gmail.com> wrote: > On Sat, Sep 9, 2017 at 7:35 AM, Cong Wang <xiyou.wangc...@gmail.com> wrote: >> On Tue, Sep 5, 2017 at 8:53 PM, Xin Long <lucien....@gmail.com> wrote: >>> Now there is no lock protecting nlk ngroups/groups' accessing in >>> netlink bind and getname. It's safe from nlk groups' setting in >>> netlink_release, but not from netlink_realloc_groups called by >>> netlink_setsockopt. >>> >>> netlink_lock_table is needed in both netlink bind and getname when >>> accessing nlk groups. >> >> This looks very odd. >> >> netlink_lock_table() should be protecting nl_table, why >> it also protects nlk->groups?? For me it looks like you >> need lock_sock() instead. > I believe netlink_lock_table might be only used to protect nl_table > at the beginning and surely lock_sock is better here. Thanks. > > But can you explain why netlink_lock_table() was also used in > netlink_getsockopt NETLINK_LIST_MEMBERSHIPS ? or it > was just a mistake ?
No, it is fine but not necessary, because netlink_realloc_groups() doesn't change nl_table, it only changes nlk->groups. So we don't have take the global write lock, the lock sock makes more sense here, same for your bind() and getname() case.
