lkml.org 
[lkml]   [2021]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.4 176/177] neighbour: Prevent Race condition in neighbour subsytem
    Date
    From: Chinmay Agarwal <chinagar@codeaurora.org>

    commit eefb45eef5c4c425e87667af8f5e904fbdd47abf upstream.

    Following Race Condition was detected:

    <CPU A, t0>: Executing: __netif_receive_skb() ->__netif_receive_skb_core()
    -> arp_rcv() -> arp_process().arp_process() calls __neigh_lookup() which
    takes a reference on neighbour entry 'n'.
    Moves further along, arp_process() and calls neigh_update()->
    __neigh_update(). Neighbour entry is unlocked just before a call to
    neigh_update_gc_list.

    This unlocking paves way for another thread that may take a reference on
    the same and mark it dead and remove it from gc_list.

    <CPU B, t1> - neigh_flush_dev() is under execution and calls
    neigh_mark_dead(n) marking the neighbour entry 'n' as dead. Also n will be
    removed from gc_list.
    Moves further along neigh_flush_dev() and calls
    neigh_cleanup_and_release(n), but since reference count increased in t1,
    'n' couldn't be destroyed.

    <CPU A, t3>- Code hits neigh_update_gc_list, with neighbour entry
    set as dead.

    <CPU A, t4> - arp_process() finally calls neigh_release(n), destroying
    the neighbour entry and we have a destroyed ntry still part of gc_list.

    Fixes: eb4e8fac00d1("neighbour: Prevent a dead entry from updating gc_list")
    Signed-off-by: Chinmay Agarwal <chinagar@codeaurora.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/core/neighbour.c | 4 ++++
    1 file changed, 4 insertions(+)

    --- a/net/core/neighbour.c
    +++ b/net/core/neighbour.c
    @@ -132,6 +132,9 @@ static void neigh_update_gc_list(struct
    write_lock_bh(&n->tbl->lock);
    write_lock(&n->lock);

    + if (n->dead)
    + goto out;
    +
    /* remove from the gc list if new state is permanent or if neighbor
    * is externally learned; otherwise entry should be on the gc list
    */
    @@ -148,6 +151,7 @@ static void neigh_update_gc_list(struct
    atomic_inc(&n->tbl->gc_entries);
    }

    +out:
    write_unlock(&n->lock);
    write_unlock_bh(&n->tbl->lock);
    }

    \
     
     \ /
      Last update: 2021-05-31 17:54    [W:3.249 / U:0.740 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site