batadv_v_elp_get_throughput is calling cfg80211_get_station with a pointer (sinfo) to some uninitialized memory on the stack. But most of the implementations behind cfg80211_get_station will not initialize sinfo to zero before manipulating it. For example, the member &struct station_info.filled is often only modified by using a read (of possibly uninitialized/random memory), an OR operation and then a write of the new value back to the original memory address. A caller without a preinitialized &struct station_info.filled can then no longer decide which parts of sinfo were filled in by cfg80211_get_station.
The caller of cfg80211_get_station must therefore take care that sinfo (or at least sinfo.filled) is initialized to zero. Otherwise, the caller may tries to read information which was not filled in and is therefore also uninitialized. In batadv_v_elp_get_throughput's case, an invalid "random" expected throughput may be saved for this neighbor and thus the B.A.T.M.A.N V algorithm may switch to non-optimal neighbors for certain destinations.
Fixes: 5c3245172c01 ("batman-adv: ELP - compute the metric based on the estimated throughput") Reported-by: Thomas Lauer holminateur@gmail.com Reported-by: Marcel Schmidt ff.z-casparistrasse@mailbox.org Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- v2:
* rewrite commit message
net/batman-adv/bat_v_elp.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c index 28687493..846316ea 100644 --- a/net/batman-adv/bat_v_elp.c +++ b/net/batman-adv/bat_v_elp.c @@ -102,6 +102,7 @@ static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh) if (!real_netdev) goto default_throughput;
+ memset(&sinfo, 0, sizeof(sinfo)); ret = cfg80211_get_station(real_netdev, neigh->addr, &sinfo);
dev_put(real_netdev);
On Mittwoch, 6. Juni 2018 07:39:51 CEST Sven Eckelmann wrote:
batadv_v_elp_get_throughput is calling cfg80211_get_station with a pointer (sinfo) to some uninitialized memory on the stack. But most of the implementations behind cfg80211_get_station will not initialize sinfo to zero before manipulating it. For example, the member &struct station_info.filled is often only modified by using a read (of possibly uninitialized/random memory), an OR operation and then a write of the new value back to the original memory address. A caller without a preinitialized &struct station_info.filled can then no longer decide which parts of sinfo were filled in by cfg80211_get_station.
The caller of cfg80211_get_station must therefore take care that sinfo (or at least sinfo.filled) is initialized to zero. Otherwise, the caller may tries to read information which was not filled in and is therefore also uninitialized. In batadv_v_elp_get_throughput's case, an invalid "random" expected throughput may be saved for this neighbor and thus the B.A.T.M.A.N V algorithm may switch to non-optimal neighbors for certain destinations.
Fixes: 5c3245172c01 ("batman-adv: ELP - compute the metric based on the estimated throughput") Reported-by: Thomas Lauer holminateur@gmail.com Reported-by: Marcel Schmidt ff.z-casparistrasse@mailbox.org Signed-off-by: Sven Eckelmann sven@narfation.org
v2:
Just got he information from Johannes that it should be changed in cfg80211:
<johill> ecsv: I guess cfg80211_get_station() would be better in case anyone else starts using it
Kind regards, Sven
b.a.t.m.a.n@lists.open-mesh.org