Does somebody have time to look at this?
Andrew
----- Forwarded message from Dan Carpenter error27@gmail.com -----
Date: Mon, 8 Mar 2010 16:07:01 +0300 From: Dan Carpenter error27@gmail.com To: andrew@lunn.ch Cc: kernel-janitors@vger.kernel.org Subject: batman: potential null dereference X-Spam-Status: No, score=2.9 required=4.0 tests=BAYES_00,FREEMAIL_FROM, RCVD_IN_BL_SPAMCOP_NET,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_WEB autolearn=no version=3.3.0
drivers/staging/batman-adv/routing.c 88 } else if ((orig_node->router == NULL) && (neigh_node != NULL)) { ^^^^^^^^^^^^^^^^^^^^^^^^^ 89 90 bat_dbg(DBG_ROUTES, 91 "Adding route towards: %pM (via %pM)\n", 92 orig_node->orig, neigh_node->addr); 93 hna_global_add_orig(orig_node, hna_buff, hna_buff_len); 94 95 /* route changed */ 96 } else { 97 bat_dbg(DBG_ROUTES, "Changing route towards: %pM (now via %pM - was via %pM)\n", orig_node->orig, neigh_node->addr, orig_node->router->addr); ^^^^^^^^^^^^^^^^^^^^^^^ 98 }
This could fail if debugging is enabled and neigh_node is null.
regards, dan carpenter
----- End forwarded message -----
Andrew Lunn wrote:
Does somebody have time to look at this? ----- Forwarded message from Dan Carpenter error27@gmail.com -----
[...]
drivers/staging/batman-adv/routing.c 88 } else if ((orig_node->router == NULL) && (neigh_node != NULL)) { ^^^^^^^^^^^^^^^^^^^^^^^^^ 89 90 bat_dbg(DBG_ROUTES, 91 "Adding route towards: %pM (via %pM)\n", 92 orig_node->orig, neigh_node->addr); 93 hna_global_add_orig(orig_node, hna_buff, hna_buff_len); 94 95 /* route changed */ 96 } else { 97 bat_dbg(DBG_ROUTES, "Changing route towards: %pM (now via %pM - was via %pM)\n", orig_node->orig, neigh_node->addr, orig_node->router->addr); ^^^^^^^^^^^^^^^^^^^^^^^ 98 }
This could fail if debugging is enabled and neigh_node is null.
It looks a little bit like checked with clang's static analyzer. This analyzer has problems to track constraints at all. This means that it doesn't catch the update_routes constraint "orig_node->router != neigh_node".
But I am also not good at tracking that kind of constraints and reported this or a similar "bug" in batmand a while ago.
So it is not a real bug, but maybe not easy to read.
Best regards, Sven
On Mon, Mar 08, 2010 at 04:15:30PM +0100, Sven Eckelmann wrote:
Andrew Lunn wrote:
Does somebody have time to look at this? ----- Forwarded message from Dan Carpenter error27@gmail.com -----
[...]
drivers/staging/batman-adv/routing.c 88 } else if ((orig_node->router == NULL) && (neigh_node != NULL)) { ^^^^^^^^^^^^^^^^^^^^^^^^^ 89 90 bat_dbg(DBG_ROUTES, 91 "Adding route towards: %pM (via %pM)\n", 92 orig_node->orig, neigh_node->addr); 93 hna_global_add_orig(orig_node, hna_buff, hna_buff_len); 94 95 /* route changed */ 96 } else { 97 bat_dbg(DBG_ROUTES, "Changing route towards: %pM (now via %pM - was via %pM)\n", orig_node->orig, neigh_node->addr, orig_node->router->addr); ^^^^^^^^^^^^^^^^^^^^^^^ 98 }
This could fail if debugging is enabled and neigh_node is null.
It looks a little bit like checked with clang's static analyzer. This analyzer has problems to track constraints at all. This means that it doesn't catch the update_routes constraint "orig_node->router != neigh_node".
But I am also not good at tracking that kind of constraints and reported this or a similar "bug" in batmand a while ago.
So it is not a real bug, but maybe not easy to read.
Yeah. I see what you mean...
regards, dan carpenter
Best regards, Sven
Dan Carpenter wrote: [...]
It looks a little bit like checked with clang's static analyzer. This analyzer has problems to track constraints at all. This means that it doesn't catch the update_routes constraint "orig_node->router != neigh_node".
But I am also not good at tracking that kind of constraints and reported this or a similar "bug" in batmand a while ago.
So it is not a real bug, but maybe not easy to read.
Yeah. I see what you mean...
Good. And thanks for the bug report. It is always good that someone looks over the code :)
Best regards, Sven
On Monday 08 March 2010 23:12:44 Andrew Lunn wrote:
Does somebody have time to look at this?
As far as I understand the question is: What happens if !orig_node->router && !neigh_node go into update_route() ?
Fairly easy to answer: Only update_routes() (note the "s") calls update_route() which checks for this case:
if (orig_node->router != neigh_node) update_route()
Not very obvious though ...
Regards, Marek
PS: Can we somehow better communicate that we have a mailing list for these kind of questions ?
b.a.t.m.a.n@lists.open-mesh.org