batman: potential null dereference
Does somebody have time to look at this?
Andrew
----- Forwarded message from Dan Carpenter <[email protected]> -----
Date: Mon, 8 Mar 2010 16:07:01 +0300
From: Dan Carpenter <[email protected]>
To: [email protected]
Cc: [email protected]
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 -----
Quoted 2 lines · Andrew Lunn
Does somebody have time to look at this?
----- Forwarded message from Dan Carpenter <[email protected]> -----
[...]
Quoted 17 lines
drivers/staging/batman-adv/routing.c88 } 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 }
Quoted 1 line
This could fail if debugging is enabled and
neigh_nodeis 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
Quoted 23 lines · Sven Eckelmann
Quoted 2 lines · Andrew Lunn
Does somebody have time to look at this?
----- Forwarded message from Dan Carpenter <[email protected]> -----[...]
Quoted 17 lines
drivers/staging/batman-adv/routing.c88 } 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 }Quoted 1 line
This could fail if debugging is enabled and
neigh_nodeis null.
Quoted 9 lines
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_routesconstraint "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
Quoted 2 lines
Best regards,
Sven
Dan Carpenter wrote:
[...]
Quoted 11 lines
Quoted 9 lines
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_routesconstraint "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
Quoted 1 line · 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 ?