Thread (2 messages) 2 messages, 2 authors, 2018-11-30

Re: [PATCH] batman-adv: fix null pointer dereference in batadv_gw_election

From: Sven Eckelmann <hidden>
Date: 2018-11-30 07:49:32
Also in: batman, lkml

On Friday, 30 November 2018 15:00:02 CET Wen Yang wrote:
This patch fixes a possible null pointer dereference in
batadv_gw_election, detected by the semantic patch
deref_null.cocci, with the following warning:

./net/batman-adv/gateway_client.c:289:15-24: ERROR: next_gw is NULL but dereferenced.
./net/batman-adv/gateway_client.c:290:15-29: ERROR: next_gw is NULL but dereferenced.
./net/batman-adv/gateway_client.c:291:15-29: ERROR: next_gw is NULL but dereferenced.
./net/batman-adv/gateway_client.c:292:15-27: ERROR: next_gw is NULL but dereferenced.
./net/batman-adv/gateway_client.c:293:15-27: ERROR: next_gw is NULL but dereferenced.
This patch is seems to be nonsensical. next_gw cannot be NULL at this point 
(let us call this location in the code "4."). Let us go through the code

    // 1. when both are NULL then it would jump out of the the function.
	if (curr_gw == next_gw)
		goto out;

    [...]

    if (curr_gw && !next_gw) {
        [...]
        // 2. this handles the only valid case when next_gw is NULL
    }  else if (!curr_gw && next_gw) {
        // 3. here we know that next_gw is not NULL and curr_gw is NULL
        // we can therefore infer that 
        [...]
    } else {
        // 4. here you try to add an ugly patch to handle a non-existing 
        // next_gw == NULL case
        [...]
    }

Let us go through all possible combinations:

        curr_gw  next_gw
    I   0        0
    II  x        0
    III 0        y
    IV  x        y

For I:   we would leave the function even at 1. and never reach 4.
For II:  would be handled by 2. and thus never reach 4.
For III: would be handled by 3. and thus never reach 4.
For IV:  This can be handled by 1. (when x == y). Or otherwise it would be
         handled by 4. but is not the next_gw == NULL case.

Please correct me (in case I missed something) but it looks to me that this 
patch should be rejected.

Kind regards,
	Sven

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help