• Florian Westphal's avatar
    netlink: don't call ->netlink_bind with table lock held · f2764bd4
    Florian Westphal authored
    When I added support to allow generic netlink multicast groups to be
    restricted to subscribers with CAP_NET_ADMIN I was unaware that a
    genl_bind implementation already existed in the past.
    
    It was reverted due to ABBA deadlock:
    
    1. ->netlink_bind gets called with the table lock held.
    2. genetlink bind callback is invoked, it grabs the genl lock.
    
    But when a new genl subsystem is (un)registered, these two locks are
    taken in reverse order.
    
    One solution would be to revert again and add a comment in genl
    referring 1e82a62f, "genetlink: remove genl_bind").
    
    This would need a second change in mptcp to not expose the raw token
    value anymore, e.g.  by hashing the token with a secret key so userspace
    can still associate subflow events with the correct mptcp connection.
    
    However, Paolo Abeni reminded me to double-check why the netlink table is
    locked in the first place.
    
    I can't find one.  netlink_bind() is already called without this lock
    when userspace joins a group via NETLINK_ADD_MEMBERSHIP setsockopt.
    Same holds for the netlink_unbind operation.
    
    Digging through the history, commit f7736080
    ("netlink: access nlk groups safely in netlink bind and getname")
    expanded the lock scope.
    
    commit 3a20773b ("net: netlink: cap max groups which will be considered in netlink_bind()")
    ... removed the nlk->ngroups access that the lock scope
    extension was all about.
    
    Reduce the lock scope again and always call ->netlink_bind without
    the table lock.
    
    The Fixes tag should be vs. the patch mentioned in the link below,
    but that one got squash-merged into the patch that came earlier in the
    series.
    
    Fixes: 4d54cc32 ("mptcp: avoid lock_fast usage in accept path")
    Link: https://lore.kernel.org/mptcp/20210213000001.379332-8-mathew.j.martineau@linux.intel.com/T/#u
    Cc: Cong Wang <xiyou.wangcong@gmail.com>
    Cc: Xin Long <lucien.xin@gmail.com>
    Cc: Johannes Berg <johannes.berg@intel.com>
    Cc: Sean Tranchetti <stranche@codeaurora.org>
    Cc: Paolo Abeni <pabeni@redhat.com>
    Cc: Pablo Neira Ayuso <pablo@netfilter.org>
    Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    f2764bd4
af_netlink.c 68.2 KB