maria-developers team mailing list archive

Thread
Date

Re: ce507903d0c: MDEV-22742 UBSAN: Many overflow issues in strings/decimal.c - runtime error: signed integer overflow: x * y cannot be represented in type 'long long int' (on optimized builds).

To: Alexey Botchkov <holyfoot@xxxxxxxxxxxx>
From: Sergei Golubchik <serg@xxxxxxxxxxx>
Date: Sun, 9 Jan 2022 14:35:07 +0100
Cc: maria-developers@xxxxxxxxxxxxxxxxxxx
In-reply-to: <mkTdL15t_LB.A.L4B.-Ou2hB@pweza>

Hi, Alexey!

On Jan 09, Alexey Botchkov wrote:
> revision-id: ce507903d0c (mariadb-10.2.40-161-gce507903d0c)
> parent(s): 0dae41637ab
> author: Alexey Botchkov
> committer: Alexey Botchkov
> timestamp: 2021-11-22 09:58:46 +0400
> message:
> 
> MDEV-22742 UBSAN: Many overflow issues in strings/decimal.c - runtime error: signed integer overflow: x * y cannot be represented in type 'long long int' (on optimized builds).
> 
> Avoid integer overflow, do the check before the calculation.
> 
> diff --git a/strings/decimal.c b/strings/decimal.c
> index 9d18a9ce52a..6249d7e097a 100644
> --- a/strings/decimal.c
> +++ b/strings/decimal.c
> @@ -1128,13 +1128,16 @@ int decimal2ulonglong(const decimal_t *from, ulonglong *to)
>  
>    for (intg=from->intg; intg > 0; intg-=DIG_PER_DEC1)
>    {
> -    ulonglong y=x;
> -    x=x*DIG_BASE + *buf++;
> -    if (unlikely(y > ((ulonglong) ULONGLONG_MAX/DIG_BASE) || x < y))
> +    if (unlikely (
> +          x >= ULONGLONG_MAX/DIG_BASE &&
> +          (x > ULONGLONG_MAX/DIG_BASE ||
> +             *buf > (dec1) (ULONGLONG_MAX%DIG_BASE))))

This took me a while. Personally I find it easier to understand an
exclusive condition, like

  x > ULONGLONG_MAX/DIG_BASE ||
  (x == ULONGLONG_MAX/DIG_BASE && *buf > (dec1) (ULONGLONG_MAX%DIG_BASE))

but it's equivalent to your version, so ok to push.

Did you check that this commit fixes all UBSAN issues mentioned in the
MDEV-22742?

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx