Re: 2.5.46-bk3: BUG in skbuff.c:178

From: Arnaldo Carvalho de Melo (acme@conectiva.com.br)
Date: Sat Nov 09 2002 - 23:18:55 EST


Em Fri, Nov 08, 2002 at 11:02:15PM +0100, Petr Vandrovec escreveu:
> On Fri, Nov 08, 2002 at 09:33:24PM +0200, Petr Vandrovec wrote:
> > On 8 Nov 02 at 12:01, Andrew Morton wrote:
 
> Patch below removes 'bucket' from arp_iter_state, and merges it to the pos.
> It is based on assumption that there is no more than 16M of entries in each
> bucket, and that NEIGH_HASHMASK + 1 + PNEIGH_HASHMASK + 1 < 127

I did that in the past, but it gets too ugly, see previous changeset in
bk tree, lemme see... 1.781.1.52, but anyway, I was aware of this bug but I
was on the run, going to Japan and back in 5 days :-\ Well, I have already
sent this one to several people, so if you could review/test it...

===== net/ipv4/arp.c 1.13 vs edited =====
--- 1.13/net/ipv4/arp.c Mon Oct 21 01:17:08 2002
+++ edited/net/ipv4/arp.c Wed Nov 6 08:00:22 2002
@@ -1140,67 +1140,122 @@
 
 struct arp_iter_state {
         int is_pneigh, bucket;
+ union {
+ struct neighbour *n;
+ struct pneigh_entry *pn;
+ } u;
 };
 
-static __inline__ struct neighbour *neigh_get_bucket(struct seq_file *seq,
- loff_t *pos)
+static struct neighbour *neigh_get_first(struct seq_file *seq)
 {
- struct neighbour *n = NULL;
         struct arp_iter_state* state = seq->private;
- loff_t l = *pos;
- int i;
 
- for (; state->bucket <= NEIGH_HASHMASK; ++state->bucket)
- for (i = 0, n = arp_tbl.hash_buckets[state->bucket]; n;
- ++i, n = n->next)
- /* Do not confuse users "arp -a" with magic entries */
- if ((n->nud_state & ~NUD_NOARP) && !l--) {
- *pos = i;
- goto out;
- }
-out:
- return n;
+ state->is_pneigh = 0;
+
+ for (state->bucket = 0;
+ state->bucket <= NEIGH_HASHMASK;
+ ++state->bucket) {
+ state->u.n = arp_tbl.hash_buckets[state->bucket];
+ while (state->u.n && !(state->u.n->nud_state & ~NUD_NOARP))
+ state->u.n = state->u.n->next;
+ if (state->u.n)
+ break;
+ }
+
+ return state->u.n;
 }
 
-static __inline__ struct pneigh_entry *pneigh_get_bucket(struct seq_file *seq,
- loff_t *pos)
+static struct neighbour *neigh_get_next(struct seq_file *seq)
 {
- struct pneigh_entry *n = NULL;
         struct arp_iter_state* state = seq->private;
- loff_t l = *pos;
- int i;
 
- for (; state->bucket <= PNEIGH_HASHMASK; ++state->bucket)
- for (i = 0, n = arp_tbl.phash_buckets[state->bucket]; n;
- ++i, n = n->next)
- if (!l--) {
- *pos = i;
- goto out;
- }
-out:
- return n;
+ for (; state->bucket <= NEIGH_HASHMASK;
+ ++state->bucket,
+ state->u.n = arp_tbl.hash_buckets[state->bucket]) {
+ if (state->u.n)
+ do {
+ state->u.n = state->u.n->next;
+ /* Don't confuse "arp -a" w/ magic entries */
+ } while (state->u.n &&
+ !(state->u.n->nud_state & ~NUD_NOARP));
+ if (state->u.n)
+ break;
+ }
+
+ return state->u.n;
+}
+
+static loff_t neigh_get_idx(struct seq_file *seq, loff_t pos)
+{
+ neigh_get_first(seq);
+ while (pos && neigh_get_next(seq))
+ --pos;
+ return pos;
+}
+
+static struct pneigh_entry *pneigh_get_first(struct seq_file *seq)
+{
+ struct arp_iter_state* state = seq->private;
+
+ state->is_pneigh = 1;
+
+ for (state->bucket = 0;
+ state->bucket <= PNEIGH_HASHMASK;
+ ++state->bucket) {
+ state->u.pn = arp_tbl.phash_buckets[state->bucket];
+ if (state->u.pn)
+ break;
+ }
+ return state->u.pn;
+}
+
+static struct pneigh_entry *pneigh_get_next(struct seq_file *seq)
+{
+ struct arp_iter_state* state = seq->private;
+
+ for (; state->bucket <= PNEIGH_HASHMASK;
+ ++state->bucket,
+ state->u.pn = arp_tbl.phash_buckets[state->bucket]) {
+ if (state->u.pn)
+ state->u.pn = state->u.pn->next;
+
+ if (state->u.pn)
+ break;
+ }
+ return state->u.pn;
 }
 
-static __inline__ void *arp_get_bucket(struct seq_file *seq, loff_t *pos)
+static loff_t pneigh_get_idx(struct seq_file *seq, loff_t pos)
 {
- void *rc = neigh_get_bucket(seq, pos);
+ pneigh_get_first(seq);
+ while (pos && pneigh_get_next(seq))
+ --pos;
+ return pos;
+}
+
+static void *arp_get_idx(struct seq_file *seq, loff_t pos)
+{
+ struct arp_iter_state* state = seq->private;
+ void *rc;
+ loff_t p;
+
+ read_lock_bh(&arp_tbl.lock);
+ p = neigh_get_idx(seq, pos);
 
- if (!rc) {
+ if (p || !state->u.n) {
                 struct arp_iter_state* state = seq->private;
 
                 read_unlock_bh(&arp_tbl.lock);
- state->is_pneigh = 1;
- state->bucket = 0;
- *pos = 0;
- rc = pneigh_get_bucket(seq, pos);
- }
+ pneigh_get_idx(seq, p);
+ rc = state->u.pn;
+ } else
+ rc = state->u.n;
         return rc;
 }
 
 static void *arp_seq_start(struct seq_file *seq, loff_t *pos)
 {
- read_lock_bh(&arp_tbl.lock);
- return *pos ? arp_get_bucket(seq, pos) : (void *)1;
+ return *pos ? arp_get_idx(seq, *pos - 1) : (void *)1;
 }
 
 static void *arp_seq_next(struct seq_file *seq, void *v, loff_t *pos)
@@ -1209,38 +1264,19 @@
         struct arp_iter_state* state;
 
         if (v == (void *)1) {
- rc = arp_get_bucket(seq, pos);
+ rc = arp_get_idx(seq, 0);
                 goto out;
         }
 
         state = seq->private;
         if (!state->is_pneigh) {
- struct neighbour *n = v;
-
- rc = n = n->next;
- if (n)
- goto out;
- *pos = 0;
- ++state->bucket;
- rc = neigh_get_bucket(seq, pos);
+ rc = neigh_get_next(seq);
                 if (rc)
                         goto out;
                 read_unlock_bh(&arp_tbl.lock);
- state->is_pneigh = 1;
- state->bucket = 0;
- *pos = 0;
- rc = pneigh_get_bucket(seq, pos);
- } else {
- struct pneigh_entry *pn = v;
-
- pn = pn->next;
- if (!pn) {
- ++state->bucket;
- *pos = 0;
- pn = pneigh_get_bucket(seq, pos);
- }
- rc = pn;
- }
+ rc = pneigh_get_first(seq);
+ } else
+ rc = pneigh_get_next(seq);
 out:
         ++*pos;
         return rc;
@@ -1291,7 +1327,6 @@
 static __inline__ void arp_format_pneigh_entry(struct seq_file *seq,
                                                struct pneigh_entry *n)
 {
-
         struct net_device *dev = n->dev;
         int hatype = dev ? dev->type : 0;
         char tbuf[16];
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Nov 15 2002 - 22:00:19 EST