* [PATCH 1/1] ipv4: off-by-one in continuation handling in /proc/net/route
@ 2015-08-13 10:21 Andy Whitcroft
2015-08-13 15:10 ` Eric Dumazet
0 siblings, 1 reply; 3+ messages in thread
From: Andy Whitcroft @ 2015-08-13 10:21 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
Cc: netdev, linux-kernel, Andy Whitcroft
When generating /proc/net/route we emit a header followed by a line for
each route. When a short read is performed we will restart this process
based on the open file descriptor. When calculating the start point we
fail to take into account that the 0th entry is the header. This leads
us to skip the first entry when doing a continuation read.
This can be easily seen with the comparison below:
while read l; do echo "$l"; done </proc/net/route >A
cat /proc/net/route >B
diff -bu A B | grep '^[+-]'
On my example machine I have approximatly 10KB of route output. There we
see the very first non-title element is lost in the while read case,
and an entry around the 8K mark in the cat case:
+wlan0 00000000 02021EAC 0003 0 0 400 00000000 0 0 0
-tun1 00C0AC0A 00000000 0001 0 0 950 00C0FFFF 0 0 0
Fix up the off-by-one when reaquiring position on continuation.
BugLink: http://bugs.launchpad.net/bugs/1483440
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
net/ipv4/fib_trie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From code inspection I belive this was introduced by the Fixes
below, but I have not tested this to confirm.
Fixes: 8be33e955cb9 ("ipv4: off-by-one in continuation handling in /proc/net/route")
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 37c4bb8..b0c6258 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -2465,7 +2465,7 @@ static struct key_vector *fib_route_get_idx(struct fib_route_iter *iter,
key = l->key + 1;
iter->pos++;
- if (pos-- <= 0)
+ if (--pos <= 0)
break;
l = NULL;
--
2.5.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] ipv4: off-by-one in continuation handling in /proc/net/route
2015-08-13 10:21 [PATCH 1/1] ipv4: off-by-one in continuation handling in /proc/net/route Andy Whitcroft
@ 2015-08-13 15:10 ` Eric Dumazet
2015-08-13 16:07 ` Alexander Duyck
0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2015-08-13 15:10 UTC (permalink / raw)
To: Andy Whitcroft, Alexander Duyck
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, linux-kernel
On Thu, 2015-08-13 at 11:21 +0100, Andy Whitcroft wrote:
> When generating /proc/net/route we emit a header followed by a line for
> each route. When a short read is performed we will restart this process
> based on the open file descriptor. When calculating the start point we
> fail to take into account that the 0th entry is the header. This leads
> us to skip the first entry when doing a continuation read.
>
> This can be easily seen with the comparison below:
>
> while read l; do echo "$l"; done </proc/net/route >A
> cat /proc/net/route >B
> diff -bu A B | grep '^[+-]'
>
> On my example machine I have approximatly 10KB of route output. There we
> see the very first non-title element is lost in the while read case,
> and an entry around the 8K mark in the cat case:
>
> +wlan0 00000000 02021EAC 0003 0 0 400 00000000 0 0 0
> -tun1 00C0AC0A 00000000 0001 0 0 950 00C0FFFF 0 0 0
>
> Fix up the off-by-one when reaquiring position on continuation.
>
> BugLink: http://bugs.launchpad.net/bugs/1483440
> Signed-off-by: Andy Whitcroft <apw@canonical.com>
> ---
> net/ipv4/fib_trie.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> From code inspection I belive this was introduced by the Fixes
> below, but I have not tested this to confirm.
>
> Fixes: 8be33e955cb9 ("ipv4: off-by-one in continuation handling in /proc/net/route")
You probably meant
Fixes: 8be33e955cb9 ("fib_trie: Fib walk rcu should take a tnode and key instead of a trie and a leaf")
CC Alexander for review/comment
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] ipv4: off-by-one in continuation handling in /proc/net/route
2015-08-13 15:10 ` Eric Dumazet
@ 2015-08-13 16:07 ` Alexander Duyck
0 siblings, 0 replies; 3+ messages in thread
From: Alexander Duyck @ 2015-08-13 16:07 UTC (permalink / raw)
To: Eric Dumazet, Andy Whitcroft, Alexander Duyck
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, linux-kernel
On 08/13/2015 08:10 AM, Eric Dumazet wrote:
> On Thu, 2015-08-13 at 11:21 +0100, Andy Whitcroft wrote:
>> When generating /proc/net/route we emit a header followed by a line for
>> each route. When a short read is performed we will restart this process
>> based on the open file descriptor. When calculating the start point we
>> fail to take into account that the 0th entry is the header. This leads
>> us to skip the first entry when doing a continuation read.
>>
>> This can be easily seen with the comparison below:
>>
>> while read l; do echo "$l"; done </proc/net/route >A
>> cat /proc/net/route >B
>> diff -bu A B | grep '^[+-]'
>>
>> On my example machine I have approximatly 10KB of route output. There we
>> see the very first non-title element is lost in the while read case,
>> and an entry around the 8K mark in the cat case:
>>
>> +wlan0 00000000 02021EAC 0003 0 0 400 00000000 0 0 0
>> -tun1 00C0AC0A 00000000 0001 0 0 950 00C0FFFF 0 0 0
>>
>> Fix up the off-by-one when reaquiring position on continuation.
>>
>> BugLink: http://bugs.launchpad.net/bugs/1483440
>> Signed-off-by: Andy Whitcroft <apw@canonical.com>
>> ---
>> net/ipv4/fib_trie.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> From code inspection I belive this was introduced by the Fixes
>> below, but I have not tested this to confirm.
>>
>> Fixes: 8be33e955cb9 ("ipv4: off-by-one in continuation handling in /proc/net/route")
> You probably meant
>
> Fixes: 8be33e955cb9 ("fib_trie: Fib walk rcu should take a tnode and key instead of a trie and a leaf")
>
> CC Alexander for review/comment
I agree the Fixes line needs to be updated. Other than that the fix
looks good. Once you get the comment fixed feel free to add my
acked-by. Also you might call out that this is needed for net, and 4.1
stable as well.
Acked-by: Alexander Duyck <alexander.h.duyck@redhat.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-08-13 16:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-13 10:21 [PATCH 1/1] ipv4: off-by-one in continuation handling in /proc/net/route Andy Whitcroft
2015-08-13 15:10 ` Eric Dumazet
2015-08-13 16:07 ` Alexander Duyck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).