[PATCH] net, batman: don't crash on zero length strings in routing_algo
Subsystems:
batman advanced , the rest
4984d
7 messages,
4 authors,
2012-12-25 · open the first message on its own page
The code that works with routing_algo assumes that the string passed is non
empty, this assumption is wrong:
sh-4.2# echo -ne '\0' > /sys/module/batman_adv/parameters/routing_algo
[ 34.531340] BUG: unable to handle kernel paging request at ffff880015142fff
[ 34.539191] IP: [<ffffffff8390ac7a>] batadv_param_set_ra+0x3a/0x90
[ 34.541128] PGD 5027063 PUD 502b063 PMD 1bfc6067 PTE 15142160
[ 34.541128] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[ 34.541128] CPU 0
[ 34.541128] Pid: 6612, comm: sh Tainted: G W 3.7.0-rc6-sasha-00024-g33da443-dirty #157
[ 34.541128] RIP: 0010:[<ffffffff8390ac7a>] [<ffffffff8390ac7a>] batadv_param_set_ra+0x3a/0x90
[ 34.541128] RSP: 0018:ffff880014f81e48 EFLAGS: 00010292
[ 34.541128] RAX: 000000000000003b RBX: ffff880015143000 RCX: 0000000000000006
[ 34.550025] RDX: 0000000000000006 RSI: ffff8800151cb960 RDI: 0000000000000282
[ 34.550025] RBP: ffff880014f81e68 R08: 0000000000000003 R09: 0000000000000000
[ 34.550025] R10: 0000000000000000 R11: 0000000000000001 R12: ffff880015142fff
[ 34.550025] R13: ffffffff84e6b390 R14: ffff880014f86a00 R15: ffffffff83c35170
[ 34.550025] FS: 00007f9ebc796700(0000) GS:ffff88001a600000(0000) knlGS:0000000000000000
[ 34.550025] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 34.550025] CR2: ffff880015142fff CR3: 000000001522f000 CR4: 00000000000406f0
[ 34.550025] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 34.550025] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 34.550025] Process sh (pid: 6612, threadinfo ffff880014f80000, task ffff8800151cb000)
[ 34.550025] Stack:
[ 34.550025] ffff880014f81e68 ffff8800198ee020 0000000000000001 ffff880015143000
[ 34.550025] ffff880014f81e98 ffffffff81133776 ffff880014f81ea8 ffff880014f86a20
[ 34.550025] ffff880014f81f50 ffff880019d86d20 ffff880014f81ea8 ffffffff811335f8
[ 34.550025] Call Trace:
[ 34.550025] [<ffffffff81133776>] param_attr_store+0x46/0x80
[ 34.550025] [<ffffffff811335f8>] module_attr_store+0x18/0x40
[ 34.550025] [<ffffffff812ed751>] sysfs_write_file+0x101/0x170
[ 34.550025] [<ffffffff8126fcb8>] vfs_write+0xb8/0x180
[ 34.550025] [<ffffffff8126fe70>] sys_write+0x50/0xa0
[ 34.550025] [<ffffffff83b30018>] tracesys+0xe1/0xe6
[ 34.550025] Code: 4c 89 65 f0 4c 89 6d f8 49 89 f5 e8 71 c5 0b fe 48 c7 c7 38 2e df 84 4c 8d 60 ff 48 89 c6 31 c0 4c 89 e2 49 01 dc e8 a6 d8 15 00 <41> 80 3c 24 0a 75 05 41 c6 04 24 00 48 89 df e8 62 ff ff ff 48
[ 34.550025] RIP [<ffffffff8390ac7a>] batadv_param_set_ra+0x3a/0x90
[ 34.550025] RSP <ffff880014f81e48>
[ 34.550025] CR2: ffff880015142fff
[ 34.550025] ---[ end trace 6c53b662c574774b ]---
Signed-off-by: Sasha Levin <redacted>
---
net/batman-adv/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index dc33a0c..3b8e368 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c @@ -426,7 +426,7 @@ static int batadv_param_set_ra(const char *val, const struct kernel_param *kp)
char * algo_name = ( char * ) val ;
size_t name_len = strlen ( algo_name );
- if ( algo_name [ name_len - 1 ] == '\n' ) + if ( name_len > 0 && algo_name [ name_len - 1 ] == '\n' ) algo_name [ name_len - 1 ] = '\0' ;
bat_algo_ops = batadv_algo_get ( algo_name ); --
1.8.0
On Mon, Nov 19, 2012 at 03:08:15PM -0500, Sasha Levin wrote: The code that works with routing_algo assumes that the string passed is non
empty, this assumption is wrong:
sh-4.2# echo -ne '\0' > /sys/module/batman_adv/parameters/routing_algo
[ 34.531340] BUG: unable to handle kernel paging request at ffff880015142fff
[CUT]
[ 34.550025] ---[ end trace 6c53b662c574774b ]---
Signed-off-by: Sasha Levin <redacted>
Hello Sasha,
thank you very much for fixing this bug!
However, any patch sent against the B.A.T.M.A.N.-Advanced code should have a
subject starting with "batman-adv:".
Other than that I think this kind of patch
doesn't really need to report the entire kernel message: a more exhaustive
commit message is enough (e.g. use function names). I personally needed to read
the patch before understanding what you were trying to explain in the message.
Then, we usually pick this patches up in our repo and then we send them as batch
to the networking tree via pull request, therefore you can also skip the netdev
ml when sending the fixes.
Thank you very much!
Regards,
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
On Monday, December 24, 2012 22:18:52 Pau Koning wrote: On Mon, Nov 19, 2012 at 9:08 PM, Sasha Levin [off-list ref] wrote: quoted The code that works with routing_algo assumes that the string passed is
non quoted empty, this assumption is wrong: Why isn't this patch part of Linux 3.7? It seems to be a bugfix and it
was sent early enough?
The patch received a reply mere 2 hours after it was sent. Again, please read
all mails before making noise.
Thanks,
Marek
On Mon, Nov 19, 2012 at 9:08 PM, Sasha Levin [off-list ref] wrote: The code that works with routing_algo assumes that the string passed is non
empty, this assumption is wrong:
Why isn't this patch part of Linux 3.7? It seems to be a bugfix and it
was sent early enough?
On Mon, Dec 24, 2012 at 3:38 PM, Marek Lindner [off-list ref] wrote: On Monday, December 24, 2012 22:18:52 Pau Koning wrote: quoted On Mon, Nov 19, 2012 at 9:08 PM, Sasha Levin [off-list ref] wrote: quoted The code that works with routing_algo assumes that the string passed is
non quoted empty, this assumption is wrong: Why isn't this patch part of Linux 3.7? It seems to be a bugfix and it
was sent early enough?
The patch received a reply mere 2 hours after it was sent. Again, please read
all mails before making noise.
Ok, leaving this problem unsolved is the correct way to handle it?
Nobody is allowed to say anything?
On Monday, December 24, 2012 23:42:15 Pau Koning wrote: On Mon, Dec 24, 2012 at 3:38 PM, Marek Lindner [off-list ref] wrote: quoted On Monday, December 24, 2012 22:18:52 Pau Koning wrote: quoted On Mon, Nov 19, 2012 at 9:08 PM, Sasha Levin [off-list ref] wrote: quoted quoted quoted The code that works with routing_algo assumes that the string passed
is non quoted empty, this assumption is wrong: Why isn't this patch part of Linux 3.7? It seems to be a bugfix and it
was sent early enough?
The patch received a reply mere 2 hours after it was sent. Again, please
read all mails before making noise.
Ok, leaving this problem unsolved is the correct way to handle it?
Nobody is allowed to say anything?
No, of course you are allowed "to say" something. I was simply pointing you to
the explanation why the patch wasn't merged yet.
Cheers,
Marek
On Tuesday, December 25, 2012 03:36:36 Marek Lindner wrote: quoted quoted quoted quoted empty, this assumption is wrong: Why isn't this patch part of Linux 3.7? It seems to be a bugfix and it
was sent early enough?
The patch received a reply mere 2 hours after it was sent. Again,
please read all mails before making noise.
Ok, leaving this problem unsolved is the correct way to handle it?
Nobody is allowed to say anything?
No, of course you are allowed "to say" something. I was simply pointing you
to the explanation why the patch wasn't merged yet.
Let me add here: Feel free to do the required cleanup work and re-submit the
patch. Nobody stops you from doing that as well. :-)
Cheers,
Marek