• Thomas Gleixner's avatar
    net: enic: Cure the enic api locking trainwreck · a53b59ec
    Thomas Gleixner authored
    enic_dev_wait() has a BUG_ON(in_interrupt()).
    
    Chasing the callers of enic_dev_wait() revealed the gems of enic_reset()
    and enic_tx_hang_reset() which are both invoked through work queues in
    order to be able to call rtnl_lock(). So far so good.
    
    After locking rtnl both functions acquire enic::enic_api_lock which
    serializes against the (ab)use from infiniband. This is where the
    trainwreck starts.
    
    enic::enic_api_lock is a spin_lock() which implicitly disables preemption,
    but both functions invoke a ton of functions under that lock which can
    sleep. The BUG_ON(in_interrupt()) does not trigger in that case because it
    can't detect the preempt disabled condition.
    
    This clearly has never been tested with any of the mandatory debug options
    for 7+ years, which would have caught that for sure.
    
    Cure it by adding a enic_api_busy member to struct enic, which is modified
    and evaluated with enic::enic_api_lock held.
    
    If enic_api_devcmd_proxy_by_index() observes enic::enic_api_busy as true,
    it drops enic::enic_api_lock and busy waits for enic::enic_api_busy to
    become false.
    
    It would be smarter to wait for a completion of that busy period, but
    enic_api_devcmd_proxy_by_index() is called with other spin locks held which
    obviously can't sleep.
    
    Remove the BUG_ON(in_interrupt()) check as well because it's incomplete and
    with proper debugging enabled the problem would have been caught from the
    debug checks in schedule_timeout().
    
    Fixes: 0b038566 ("drivers/net: enic: Add an interface for USNIC to interact with firmware")
    Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    a53b59ec
enic_api.c 1.57 KB