All of lore.kernel.org
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] FWD: batman: potential null dereference
@ 2010-03-08 15:12 Andrew Lunn
  2010-03-08 15:15 ` Sven Eckelmann
  2010-03-08 15:21 ` Marek Lindner
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Lunn @ 2010-03-08 15:12 UTC (permalink / raw)
  To: B.A.T.M.A.N

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 -----

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [B.A.T.M.A.N.] FWD: batman: potential null dereference
  2010-03-08 15:12 [B.A.T.M.A.N.] FWD: batman: potential null dereference Andrew Lunn
@ 2010-03-08 15:15 ` Sven Eckelmann
  2010-03-08 16:11   ` Dan Carpenter
  2010-03-08 15:21 ` Marek Lindner
  1 sibling, 1 reply; 6+ messages in thread
From: Sven Eckelmann @ 2010-03-08 15:15 UTC (permalink / raw)
  To: B.A.T.M.A.N; +Cc: Dan Carpenter

[-- Attachment #1: Type: Text/Plain, Size: 1429 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [B.A.T.M.A.N.] FWD: batman: potential null dereference
  2010-03-08 15:12 [B.A.T.M.A.N.] FWD: batman: potential null dereference Andrew Lunn
  2010-03-08 15:15 ` Sven Eckelmann
@ 2010-03-08 15:21 ` Marek Lindner
  1 sibling, 0 replies; 6+ messages in thread
From: Marek Lindner @ 2010-03-08 15:21 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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 ?


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [B.A.T.M.A.N.] FWD: batman: potential null dereference
  2010-03-08 15:15 ` Sven Eckelmann
@ 2010-03-08 16:11   ` Dan Carpenter
  2010-03-08 16:23       ` Sven Eckelmann
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2010-03-08 16:11 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: B.A.T.M.A.N

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [B.A.T.M.A.N.] FWD: batman: potential null dereference
@ 2010-03-08 16:23       ` Sven Eckelmann
  0 siblings, 0 replies; 6+ messages in thread
From: Sven Eckelmann @ 2010-03-08 16:23 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: kernel-janitors, B.A.T.M.A.N

[-- Attachment #1: Type: Text/Plain, Size: 623 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [B.A.T.M.A.N.] FWD: batman: potential null dereference
@ 2010-03-08 16:23       ` Sven Eckelmann
  0 siblings, 0 replies; 6+ messages in thread
From: Sven Eckelmann @ 2010-03-08 16:23 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: Text/Plain, Size: 623 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-03-08 16:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-08 15:12 [B.A.T.M.A.N.] FWD: batman: potential null dereference Andrew Lunn
2010-03-08 15:15 ` Sven Eckelmann
2010-03-08 16:11   ` Dan Carpenter
2010-03-08 16:23     ` Sven Eckelmann
2010-03-08 16:23       ` Sven Eckelmann
2010-03-08 15:21 ` Marek Lindner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.