batman-adv: tp_meter: avoid divide-by-zero for dec_cwnd

The cwnd is always MSS <= cwnd <= 0x20000000. But the calculation in
batadv_tp_update_cwnd() assumes unsigned 32 bit arithmetics.

    ((mss * 8) ** 2) / (cwnd * 8)

In case cwnd is actually 0x20000000, it will be shifted by 3 bit to the
left end up at 0x100000000 or U32_MAX + 1. It will therefore wrap around
and be 0 - resulting in:

    ((mss * 8) ** 2) / 0

This is of course invalid and cannot be calculated. The calculation should
must be simplified to avoid this overflow:

   (mss ** 2) * 8 / cwnd

It will keep the precision enhancement from the scaling (by 8) but avoid
the overflow in the divisor.

In theory, there could still be an overflow in the dividend. It is at the
moment fixed to BATADV_TP_PLEN in batadv_tp_recv_ack() - so it is not an
imminent problem. But allowing it to use the whole u32 bit range, would
mean that it can still use up to 67 bits. To keep this calculation safe for
32 bit arithmetic, mss must never use more than floor((32 - 3) / 2) bits -
or in other words: must never be larger than 16383.

Cc: stable@kernel.org
Fixes: 33a3bb4a3345 ("batman-adv: throughput meter implementation")
Signed-off-by: Sven Eckelmann <sven@narfation.org>

diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c
index 9ecbc6023cfc9822cf7a98a3c46b9637908f6133..1655f181c92937113a118c61c389867ec0d0ca47 100644 (file)
--- a/net/batman-adv/tp_meter.c
+++ b/net/batman-adv/tp_meter.c
@@ -154,9 +154,12 @@ static void batadv_tp_update_cwnd(struct batadv_tp_vars *tp_vars, u32 mss)
                return;
        }
 
+       /* prevent overflow in (mss * mss) << 3 */
+       mss = min_t(u32, mss, (1U << 14) - 1);
+
        /* increment CWND at least of 1 (section 3.1 of RFC5681) */
        tp_vars->dec_cwnd += max_t(u32, 1U << 3,
-                                  ((mss * mss) << 6) / (tp_vars->cwnd << 3));
+                                  ((mss * mss) << 3) / tp_vars->cwnd);
        if (tp_vars->dec_cwnd < (mss << 3)) {
                spin_unlock_bh(&tp_vars->cwnd_lock);
                return;