netdev - Re: [PATCH v2 net-next] neighbor: Improve garbage collection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0e9345e1-fbed-3f36-e11f-0826188521f0@gmail.com>
Date:   Sun, 9 Dec 2018 21:14:39 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     David Miller <davem@...emloft.net>, dsahern@...nel.org
Cc:     netdev@...r.kernel.org, roopa@...ulusnetworks.com,
        dsahern@...il.com
Subject: Re: [PATCH v2 net-next] neighbor: Improve garbage collection



On 12/09/2018 09:12 PM, Eric Dumazet wrote:

> What protects gc_list linkage ?
> 
> We can not use list_del_init(&n->gc_list);                    or 
>                list_add_tail(&n->gc_list, &n->tbl->gc_list);
> 
> if tbl->lock is not held.
> 
> It seems to me this patch needs more care.
> 

I am playing with a LOCKDEP assist :

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index c3b58712e98b9157ca717951da40eb5ae2fe810b..213e56a3816918e599f9a658bcb851711c578f7b 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -122,6 +122,7 @@ static void neigh_mark_dead(struct neighbour *n)
 {
        n->dead = 1;
        if (!list_empty(&n->gc_list)) {
+               WARN_ON_ONCE(debug_locks && !lockdep_is_held(&n->tbl->lock));
                list_del_init(&n->gc_list);
                atomic_dec(&n->tbl->gc_entries);
        }
@@ -138,10 +139,12 @@ static void neigh_change_state(struct neighbour *n, u8 new)
         * add to the gc list if new state is not permanent
         */
        if (new_is_perm && on_gc_list) {
+               WARN_ON_ONCE(debug_locks && !lockdep_is_held(&n->tbl->lock));
                list_del_init(&n->gc_list);
                atomic_dec(&n->tbl->gc_entries);
        } else if (!new_is_perm && !on_gc_list) {
                /* add entries to the tail; cleaning removes from the front */
+               WARN_ON_ONCE(debug_locks && !lockdep_is_held(&n->tbl->lock));
                list_add_tail(&n->gc_list, &n->tbl->gc_list);
                atomic_inc(&n->tbl->gc_entries);
        }
@@ -391,8 +394,10 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl,
        refcount_set(&n->refcnt, 1);
        n->dead           = 1;
 
-       if (!permanent)
+       if (!permanent) {
+               WARN_ON_ONCE(debug_locks && !lockdep_is_held(&n->tbl->lock));
                list_add_tail(&n->gc_list, &n->tbl->gc_list);
+       }
        else
                INIT_LIST_HEAD(&n->gc_list);