mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Don Law <opendon@donlaw.com>
To: davem@davemloft.net, kuznet@ms2.inr.ac.ru, pekkas@netcore.fi,
	jmorris@namei.org, yoshfuji@linux-ipv6.org, kaber@coreworks.de
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH linux-2.6.17-rc1] net: fix neigh_delete to handle mult. tables
Date: Thu, 20 Apr 2006 23:31:24 -0400	[thread overview]
Message-ID: <4448520C.5080508@donlaw.com> (raw)

The neigh_delete assumes that there is only one table with the desired
protocol family for the address being deleted.  This assumption is not
always valid.  For example, in a kernel configured with both the
typical Ethernet configuration (CONFIG_NET_ETHERNET=y etc.) as well
as ATM (CONFIG_ATM=y and CONFIG_ATM_CLIP=y), there are two tables, 
namely arp_tbl and clip_tbl, both with family AF_INET.

The problem fixed by this patch is that the "for" loop in neigh_delete
will not iterate further once it finds one table with a matching family.
The code changes refactor neigh_delete such that it will continue
to iterate through the neighbor tables until it finds a match or 
finishes the tables.

To see the symptom of the problem, you must have a kernel with more
than one AF_INET transport configured, then try to delete a neighbor
table entry.  The delete will fail with EINVAL.  For example:

    [root@bonefish ~]# ip neigh list dev eth4
    192.168.4.24 lladdr 00:90:fb:a1:07:2f nud reachable
    [root@bonefish ~]# ip neigh delete 192.168.4.24 dev eth4
    RTNETLINK answers: Invalid argument
    [root@bonefish ~]# ip neigh list dev eth4
    192.168.4.24 lladdr 00:90:fb:a1:07:2f nud reachable

One side effect of this change is that neigh_lookup is now called while
a read lock is held on neigh_tbl_lock.  neigh_lookup does take read
locks on the tbl->lock set.  However, this does not introduce any new lock
order dependencies, since we already have that precedent set in the
neightbl_set function.

Signed-off-by: Don Law <opendon@donlaw.com>

---

I have built this patch against the 2.6.17-rc1 kernel.

I tested this patch with the simple test case described above, plus let
it run for a few days on the box to make sure it behaves OK.  I also
verified that I can properly delete IPV6 neighbor entries.


--- linux-2.6.17-rc1/net/core/neighbour.c.orig  2006-04-11 16:28:10.000000000 -0
400
+++ linux-2.6.17-rc1/net/core/neighbour.c       2006-04-13 10:01:19.000000000 -0
400
@@ -1430,48 +1430,54 @@ int neigh_delete(struct sk_buff *skb, st
        struct rtattr **nda = arg;
        struct neigh_table *tbl;
        struct net_device *dev = NULL;
-       int err = -ENODEV;
+       int err;
 
        if (ndm->ndm_ifindex &&
-           (dev = dev_get_by_index(ndm->ndm_ifindex)) == NULL)
-               goto out;
+                       (dev = dev_get_by_index(ndm->ndm_ifindex)) == NULL)
+               return -ENODEV;
 
        read_lock(&neigh_tbl_lock);
-       for (tbl = neigh_tables; tbl; tbl = tbl->next) {
+       for (tbl = neigh_tables; ; tbl = tbl->next) {
                struct rtattr *dst_attr = nda[NDA_DST - 1];
                struct neighbour *n;
 
+               if (!tbl) {
+                       read_unlock(&neigh_tbl_lock);
+                       err = -EADDRNOTAVAIL;
+                       break;
+               }
                if (tbl->family != ndm->ndm_family)
                        continue;
-               read_unlock(&neigh_tbl_lock);
 
                err = -EINVAL;
-               if (!dst_attr || RTA_PAYLOAD(dst_attr) < tbl->key_len)
-                       goto out_dev_put;
+               if (!dst_attr || RTA_PAYLOAD(dst_attr) < tbl->key_len) {
+                       read_unlock(&neigh_tbl_lock);
+                       break;
+               }
 
                if (ndm->ndm_flags & NTF_PROXY) {
+                       read_unlock(&neigh_tbl_lock);
                        err = pneigh_delete(tbl, RTA_DATA(dst_attr), dev);
-                       goto out_dev_put;
+                       break;
                }
 
-               if (!dev)
-                       goto out;
+               if (!dev) {
+                       read_unlock(&neigh_tbl_lock);
+                       break;
+               }
 
                n = neigh_lookup(tbl, RTA_DATA(dst_attr), dev);
                if (n) {
-                       err = neigh_update(n, NULL, NUD_FAILED, 
+                       read_unlock(&neigh_tbl_lock);
+                       err = neigh_update(n, NULL, NUD_FAILED,
                                           NEIGH_UPDATE_F_OVERRIDE|
                                           NEIGH_UPDATE_F_ADMIN);
                        neigh_release(n);
+                       break;
                }
-               goto out_dev_put;
        }
-       read_unlock(&neigh_tbl_lock);
-       err = -EADDRNOTAVAIL;
-out_dev_put:
        if (dev)
                dev_put(dev);
-out:
        return err;
 }



             reply	other threads:[~2006-04-21  3:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-04-21  3:31 Don Law [this message]
2006-04-21  3:55 ` YOSHIFUJI Hideaki / 吉藤英明
2006-04-21  4:20   ` YOSHIFUJI Hideaki / 吉藤英明
  -- strict thread matches above, loose matches on Subject: below --
2006-04-14  3:42 Don Law

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4448520C.5080508@donlaw.com \
    --to=opendon@donlaw.com \
    --cc=davem@davemloft.net \
    --cc=jmorris@namei.org \
    --cc=kaber@coreworks.de \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pekkas@netcore.fi \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®