| Message ID | 91362fa6-46df-c134-63b1-cc2b0d2832ee@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Stephen Hemminger |
| Headers | show |
| Series | [iproute2] ip/ipnexthop: fix unsigned overflow in parse_nh_group_type_res() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Wed, 17 Nov 2021 22:11:24 +0300 Maxim Petrov <mmrmaximuzz@gmail.com> wrote: > 0UL has type 'unsigned long' which is likely to be 64bit on modern machines. At > the same time, the '{idle,unbalanced}_timer' variables are declared as u32, so > these variables cannot be greater than '~0UL / 100' when 'unsigned long' is 64 > bits. In such condition it is still possible to pass the check but get the > overflow later when the timers are multiplied by 100 in 'addattr32'. > > Fix the possible overflow by changing '~0UL' to 'UINT32_MAX'. > > Signed-off-by: Maxim Petrov <mmrmaximuzz@gmail.com> Fixes: 91676718228b ("nexthop: Add support for resilient nexthop groups")
On Wed, Nov 17, 2021 at 10:11:24PM +0300, Maxim Petrov wrote: > 0UL has type 'unsigned long' which is likely to be 64bit on modern machines. At > the same time, the '{idle,unbalanced}_timer' variables are declared as u32, so > these variables cannot be greater than '~0UL / 100' when 'unsigned long' is 64 > bits. In such condition it is still possible to pass the check but get the > overflow later when the timers are multiplied by 100 in 'addattr32'. > > Fix the possible overflow by changing '~0UL' to 'UINT32_MAX'. > > Signed-off-by: Maxim Petrov <mmrmaximuzz@gmail.com> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c index 83a5540e..2f448449 100644 --- a/ip/ipnexthop.c +++ b/ip/ipnexthop.c @@ -6,6 +6,7 @@ */ #include <linux/nexthop.h> +#include <stdint.h> #include <stdio.h> #include <string.h> #include <rt_names.h> @@ -840,7 +841,7 @@ static void parse_nh_group_type_res(struct nlmsghdr *n, int maxlen, int *argcp, NEXT_ARG(); if (get_unsigned(&idle_timer, *argv, 0) || - idle_timer >= ~0UL / 100) + idle_timer >= UINT32_MAX / 100) invarg("invalid idle timer value", *argv); addattr32(n, maxlen, NHA_RES_GROUP_IDLE_TIMER, @@ -850,7 +851,7 @@ static void parse_nh_group_type_res(struct nlmsghdr *n, int maxlen, int *argcp, NEXT_ARG(); if (get_unsigned(&unbalanced_timer, *argv, 0) || - unbalanced_timer >= ~0UL / 100) + unbalanced_timer >= UINT32_MAX / 100) invarg("invalid unbalanced timer value", *argv); addattr32(n, maxlen, NHA_RES_GROUP_UNBALANCED_TIMER,
0UL has type 'unsigned long' which is likely to be 64bit on modern machines. At the same time, the '{idle,unbalanced}_timer' variables are declared as u32, so these variables cannot be greater than '~0UL / 100' when 'unsigned long' is 64 bits. In such condition it is still possible to pass the check but get the overflow later when the timers are multiplied by 100 in 'addattr32'. Fix the possible overflow by changing '~0UL' to 'UINT32_MAX'. Signed-off-by: Maxim Petrov <mmrmaximuzz@gmail.com> --- ip/ipnexthop.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)