[lvc-project] [PATCH] atm: horizon: Casting type 32 to 64 bits in make_rate().

[Fedor Pchelkin]

On Tue, 15. Oct 08:19, Andrey Shumilin wrote:
> In one of the 3 cases, 1<<30 is passed as the second
> parameter to the make_rate() function.
> In the expressions "c << (CR_MAXPEXP+div-br_exp)"
> and "c<<div" a shift of 14 is possible.
> The INT type may overflow.

Для "c" используется беззнаковый тип u32 - это важно. Согласно стандарту
undefined behavior при сдвиговом переполнении нет.
  The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits
  are filled with zeros. If E1 has an unsigned type, the value of the
  result is E1*2^E2, reduced modulo one more than the maximum value
  representable in the result type. (C99, 6.5.7)

> To fix this, it is suggested to cast the type.

Очень короткие строки в абзаце.

> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Andrey Shumilin <shum.sdl at nppct.ru>
> ---
>  drivers/atm/horizon.c | 4 ++--

Настоятельно рекомендую убрать этот модуль из Вашей конфигурации..

В upstream код удалён - коммит 5b74a20d35ab ("net: atm: remove support for
Madge Horizon ATM devices").

>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/atm/horizon.c b/drivers/atm/horizon.c
> index 4f2951cbe69c..6f3e65e65225 100644
> --- a/drivers/atm/horizon.c
> +++ b/drivers/atm/horizon.c
> @@ -631,7 +631,7 @@ static int make_rate (const hrz_dev * dev, u32 c, rounding r,
>  	// d == MIND and (c << (MAXPEXP+MIND)) < B
>  	while (div < CR_MAXD) {
>  		div++;
> -		if (br_man <= (c << (CR_MAXPEXP+div-br_exp))) {

Здесь беззнаковое переполнение используется намеренно - видимо часть
алгоритма, который повсюду в комментариях внутри и к данной функции
описывается.

В Svacer предлагаю выставить Won't fix с данной аргументацией.

> +		if (br_man <= ((u64)c << (CR_MAXPEXP+div-br_exp))) {
>  			// Equivalent to: B <= (c << (MAXPEXP+d))
>  			// c << (MAXPEXP+d-1) < B <= c << (MAXPEXP+d)
>  			// 1 << (MAXPEXP-1) < B/2^d/c <= 1 << MAXPEXP
> @@ -645,7 +645,7 @@ static int make_rate (const hrz_dev * dev, u32 c, rounding r,
>  					pre = DIV_ROUND_CLOSEST(br, c<<div);
>  					break;
>  				default: /* round_up */
> -					pre = br/(c<<div);
> +					pre = br/((u64)c<<div);
>  			}
>  			PRINTD (DBG_QOS, "B: p=%u, d=%u", pre, div);
>  			goto got_it;
> -- 
> 2.30.2