[PATCH] net, batman: don't crash on zero length strings in routing_algo

[PATCH] net, batman: don't crash on zero length strings in routing_algo

[Sasha Levin]

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 <sasha.levin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 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

Re: [PATCH] net, batman: don't crash on zero length strings in routing_algo

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]

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 <sasha.levin@oracle.com>


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

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] net, batman: don't crash on zero length strings in routing_algo

[Pau Koning]

On Mon, Nov 19, 2012 at 9:08 PM, Sasha Levin <sasha.levin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 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?

Re: [PATCH] net, batman: don't crash on zero length strings in routing_algo

[Marek Lindner]

On Monday, December 24, 2012 22:18:52 Pau Koning wrote:
> On Mon, Nov 19, 2012 at 9:08 PM, Sasha Levin <sasha.levin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 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?

The patch received a reply mere 2 hours after it was sent. Again, please read 
all mails before making noise.

Thanks,
Marek

Re: [PATCH] net, batman: don't crash on zero length strings in routing_algo

[Pau Koning]

On Mon, Dec 24, 2012 at 3:38 PM, Marek Lindner <lindner_marek-LWAfsSFWpa4@public.gmane.org> wrote:
> On Monday, December 24, 2012 22:18:52 Pau Koning wrote:
>> On Mon, Nov 19, 2012 at 9:08 PM, Sasha Levin <sasha.levin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 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?
>
> 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?

Re: [PATCH] net, batman: don't crash on zero length strings in routing_algo

[Marek Lindner]

On Monday, December 24, 2012 23:42:15 Pau Koning wrote:
> On Mon, Dec 24, 2012 at 3:38 PM, Marek Lindner <lindner_marek-LWAfsSFWpa4@public.gmane.org> 
wrote:
> > On Monday, December 24, 2012 22:18:52 Pau Koning wrote:
> >> On Mon, Nov 19, 2012 at 9:08 PM, Sasha Levin <sasha.levin-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 
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?
> > 
> > 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

Re: [PATCH] net, batman: don't crash on zero length strings in routing_algo

[Marek Lindner]

On Tuesday, December 25, 2012 03:36:36 Marek Lindner wrote:
> > >> > 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