[PATCH] net/ipv4/route.c, kernel 2.4.20

[PATCH] net/ipv4/route.c, kernel 2.4.20

[Arcady Stepanov]

Hello, all.

After upgrading a kernel from 2.4.19 to 2.4.20, I notice a bug
in ip_rt_acct_read() @ net/ipv4/route.c causing incorrect results
when reading route accounting information
from /proc/net/rt_acct pseudofile. Look at this example output from rtacct
utility:

[penguin@hq2 penguin]$ rtacct
Realm      BytesTo    PktsTo     BytesFrom  PktsFrom
unknown    2525573046 17491813   2696204488 15870526
macom.rus  2287217919 13724644   1092230247 14569843
macom.def  1670916923 15376790   3028924585 16093284
mol.arbat  476286103  884267     380803784  827802
mol.core   758578958  5083436    428582889  5382495
mol.dip    1953973432 3800121    279550530  3437570

-- [skipped] --

192        2525573046 17491813   2696204488 15870526
194        2287217919 13724644   1092230247 14569843
195        1670916923 15376790   3028924585 16093284
199        476286103  884267     380803784  827802
200        758578958  5083436    428582889  5382495
201        1953973432 3800121    279550530  3437570

-- [skipped] --

As you can see, all realms with numbers < 192 has a "twins" with
>= 192 with exactly same statistics data. Statistics for _real_ realms with 
numbers >= 192 becomes inaccessible. These results caused
by ip_rt_acct_read function, which has been completely owerwritten
in 2.4.20: New code completely ignores offset argument when copying stats data
in buffer.

The patch is trivial:

--- net/ipv4/route.c.orig	Sun Jan  5 15:29:10 2003
+++ net/ipv4/route.c	Fri Apr 18 10:11:23 2003
@@ -2422,7 +2422,7 @@
 /* This code sucks.  But you should have seen it before! --RR */
 
 /* IP route accounting ptr for this logical cpu number. */
-#define IP_RT_ACCT_CPU(i) (ip_rt_acct + cpu_logical_map(i) * 256)
+#define IP_RT_ACCT_CPU(i) ((u8*)ip_rt_acct + cpu_logical_map(i) * 256)
 
 static int ip_rt_acct_read(char *buffer, char **start, off_t offset,
 			   int length, int *eof, void *data)
@@ -2442,17 +2442,22 @@
 		*eof = 1;
 	}
 
-	/* Copy first cpu. */
 	*start = buffer;
-	memcpy(buffer, IP_RT_ACCT_CPU(0), length);
 
-	/* Add the other cpus in, one int at a time */
-	for (i = 1; i < smp_num_cpus; i++) {
-		unsigned int j;
-		for (j = 0; j < length/4; j++)
-			((u32*)buffer)[j] += ((u32*)IP_RT_ACCT_CPU(i))[j];
+	if (length > 0)
+	{
+		/* Copy first cpu. */
+		memcpy(buffer, IP_RT_ACCT_CPU(0) + offset, length);
+
+		/* Add the other cpus in, one int at a time */
+		for (i = 1; i < smp_num_cpus; i++) {
+			unsigned int j;
+			for (j = 0; j < length/4; j++)
+				((u32*)buffer)[j] += ((u32*)(IP_RT_ACCT_CPU(i) + offset))[j];
+		}
+		return length;
 	}
-	return length;
+	return 0;
 }
 #endif
 
It also cures a bug in pointer aritmetics in IP_RT_ACCT_CPU macro, which
will cause incorrect statistics output on SMP systems.

-- 
   BW, Arcady Stepanov (AS713-RIPE, AS28-RIPN).

   Micronic on-line Network Operating Center.
   email:noc@mol.ru; phone: +7 095 2320012

==========================================
 All that we can do is just survive
 All that we can do to help ourselves
 Is stay alive...
 
    Rush, "Red Sector A"
==========================================

Re: [PATCH] net/ipv4/route.c, kernel 2.4.20

[David S. Miller]

On Fri, 2003-04-18 at 04:22, Arcady Stepanov wrote:
> The patch is trivial:

This patch is wrong.

>  /* IP route accounting ptr for this logical cpu number. */
> -#define IP_RT_ACCT_CPU(i) (ip_rt_acct + cpu_logical_map(i) * 256)
> +#define IP_RT_ACCT_CPU(i) ((u8*)ip_rt_acct + cpu_logical_map(i) * 256)

There is no way this can be correct.  Look at the formula used
by ip_input.c, which is:

  struct ip_rt_acct *st = ip_rt_acct + 256*smp_processor_id();

So there is only two possibilities, either your patch is wrong
or this code in ip_input.c is wrong.

You probably want to keep IP_RT_ACCT_CPU() how it is, cast _THAT_
to "u8 *" then add in the offset.

Also, you'd get a response more quickly if you had sent this to
the networking development lists (linux-net and netdev@oss.sgi.com)
CC:'ing the networking maintainers (myself and Alexey Kuznetsov).
I came across this posting by pure luck.

linux-kernel is effectively /dev/null for some of us on many days.
Just like ipv4 and ipv6, linux kernel is an unreliable transport.

-- 
David S. Miller <davem@redhat.com>