| Message ID | 20210421194212.GA5676@chinagar-linux.qualcomm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | eefb45eef5c4c425e87667af8f5e904fbdd47abf |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | neighbour: Prevent Race condition in neighbour subsytem | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | fail | 2 blamed authors not CCed: dsahern@kernel.org kuba@kernel.org; 7 maintainers not CCed: dsahern@kernel.org mrv@mojatatu.com jdike@akamai.com kuba@kernel.org trix@redhat.com liuhangbin@gmail.com weichen.chen@linux.alibaba.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 3 this patch: 3 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | fail | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 16 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 3 this patch: 3 |
| netdev/header_inline | success | Link |
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Thu, 22 Apr 2021 01:12:22 +0530 you wrote: > 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. > > [...] Here is the summary with links: - neighbour: Prevent Race condition in neighbour subsytem https://git.kernel.org/netdev/net/c/eefb45eef5c4 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/net/core/neighbour.c b/net/core/neighbour.c index c26ecbe..bce395ed 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -133,6 +133,9 @@ static void neigh_update_gc_list(struct neighbour *n) 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 */ @@ -149,6 +152,7 @@ static void neigh_update_gc_list(struct neighbour *n) atomic_inc(&n->tbl->gc_entries); } +out: write_unlock(&n->lock); write_unlock_bh(&n->tbl->lock); }
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> --- net/core/neighbour.c | 4 ++++ 1 file changed, 4 insertions(+)