lkml.org 
[lkml]   [2021]   [Jun]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.10 019/131] netlink: disable IRQs for netlink_lock_table()
    Date
    From: Johannes Berg <johannes.berg@intel.com>

    [ Upstream commit 1d482e666b8e74c7555dbdfbfb77205eeed3ff2d ]

    Syzbot reports that in mac80211 we have a potential deadlock
    between our "local->stop_queue_reasons_lock" (spinlock) and
    netlink's nl_table_lock (rwlock). This is because there's at
    least one situation in which we might try to send a netlink
    message with this spinlock held while it is also possible to
    take the spinlock from a hardirq context, resulting in the
    following deadlock scenario reported by lockdep:

    CPU0 CPU1
    ---- ----
    lock(nl_table_lock);
    local_irq_disable();
    lock(&local->queue_stop_reason_lock);
    lock(nl_table_lock);
    <Interrupt>
    lock(&local->queue_stop_reason_lock);

    This seems valid, we can take the queue_stop_reason_lock in
    any kind of context ("CPU0"), and call ieee80211_report_ack_skb()
    with the spinlock held and IRQs disabled ("CPU1") in some
    code path (ieee80211_do_stop() via ieee80211_free_txskb()).

    Short of disallowing netlink use in scenarios like these
    (which would be rather complex in mac80211's case due to
    the deep callchain), it seems the only fix for this is to
    disable IRQs while nl_table_lock is held to avoid hitting
    this scenario, this disallows the "CPU0" portion of the
    reported deadlock.

    Note that the writer side (netlink_table_grab()) already
    disables IRQs for this lock.

    Unfortunately though, this seems like a huge hammer, and
    maybe the whole netlink table locking should be reworked.

    Reported-by: syzbot+69ff9dff50dcfe14ddd4@syzkaller.appspotmail.com
    Signed-off-by: Johannes Berg <johannes.berg@intel.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    net/netlink/af_netlink.c | 6 ++++--
    1 file changed, 4 insertions(+), 2 deletions(-)

    diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
    index daca50d6bb12..e527f5686e2b 100644
    --- a/net/netlink/af_netlink.c
    +++ b/net/netlink/af_netlink.c
    @@ -453,11 +453,13 @@ void netlink_table_ungrab(void)
    static inline void
    netlink_lock_table(void)
    {
    + unsigned long flags;
    +
    /* read_lock() synchronizes us to netlink_table_grab */

    - read_lock(&nl_table_lock);
    + read_lock_irqsave(&nl_table_lock, flags);
    atomic_inc(&nl_table_users);
    - read_unlock(&nl_table_lock);
    + read_unlock_irqrestore(&nl_table_lock, flags);
    }

    static inline void
    --
    2.30.2


    \
     
     \ /
      Last update: 2021-06-14 13:07    [W:4.123 / U:24.800 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site