* [PATCH] batman-adv: fix null pointer dereference in batadv_gw_election
@ 2018-11-30 7:00 Wen Yang
2018-11-30 7:41 ` [B.A.T.M.A.N.] " Sven Eckelmann
0 siblings, 1 reply; 2+ messages in thread
From: Wen Yang @ 2018-11-30 7:00 UTC (permalink / raw)
To: mareklindner, sw, a, davem
Cc: netdev, b.a.t.m.a.n, linux-kernel, zhong.weidong, wen.yang99
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.
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Reviewed-by: Tan Hu <tan.hu@zte.com.cn>
---
net/batman-adv/gateway_client.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index 140c61a..d80ef1c 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -284,14 +284,16 @@ void batadv_gw_election(struct batadv_priv *bat_priv)
batadv_throw_uevent(bat_priv, BATADV_UEV_GW, BATADV_UEV_ADD,
gw_addr);
} else {
- batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
- "Changing route to gateway %pM (bandwidth: %u.%u/%u.%u MBit, tq: %i)\n",
- next_gw->orig_node->orig,
- next_gw->bandwidth_down / 10,
- next_gw->bandwidth_down % 10,
- next_gw->bandwidth_up / 10,
- next_gw->bandwidth_up % 10,
- router_ifinfo->bat_iv.tq_avg);
+ if (next_gw) {
+ batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
+ "Changing route to gateway %pM (bandwidth: %u.%u/%u.%u MBit, tq: %i)\n",
+ next_gw->orig_node->orig,
+ next_gw->bandwidth_down / 10,
+ next_gw->bandwidth_down % 10,
+ next_gw->bandwidth_up / 10,
+ next_gw->bandwidth_up % 10,
+ router_ifinfo->bat_iv.tq_avg);
+ }
batadv_throw_uevent(bat_priv, BATADV_UEV_GW, BATADV_UEV_CHANGE,
gw_addr);
}
--
2.9.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: fix null pointer dereference in batadv_gw_election
2018-11-30 7:00 [PATCH] batman-adv: fix null pointer dereference in batadv_gw_election Wen Yang
@ 2018-11-30 7:41 ` Sven Eckelmann
0 siblings, 0 replies; 2+ messages in thread
From: Sven Eckelmann @ 2018-11-30 7:41 UTC (permalink / raw)
To: b.a.t.m.a.n
Cc: Wen Yang, mareklindner, sw, a, davem, netdev, zhong.weidong,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1937 bytes --]
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
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-11-30 7:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-30 7:00 [PATCH] batman-adv: fix null pointer dereference in batadv_gw_election Wen Yang
2018-11-30 7:41 ` [B.A.T.M.A.N.] " Sven Eckelmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).