netdev - Re: [net-next 09/17] can: length: can_fd_len2dlc(): simplify length calculcation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 Jan 2021 10:59:27 +0900
From:   Vincent MAILHOL <mailhol.vincent@...adoo.fr>
To:     Marc Kleine-Budde <mkl@...gutronix.de>
Cc:     netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        linux-can <linux-can@...r.kernel.org>, kernel@...gutronix.de
Subject: Re: [net-next 09/17] can: length: can_fd_len2dlc(): simplify length calculcation

On Tue. 14 Jan 2021 at 06:14, Marc Kleine-Budde <mkl@...gutronix.de> wrote:
>
> If the length paramter in len2dlc() exceeds the size of the len2dlc array, we
> return 0xF. This is equal to the last 16 members of the array.
>
> This patch removes these members from the array, uses ARRAY_SIZE() for the
> length check, and returns CANFD_MAX_DLC (which is 0xf).
>
> Reviewed-by: Vincent Mailhol <mailhol.vincent@...adoo.fr>
> Link: https://lore.kernel.org/r/20210111141930.693847-9-mkl@pengutronix.de
> Signed-off-by: Marc Kleine-Budde <mkl@...gutronix.de>
> ---
>  drivers/net/can/dev/length.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/can/dev/length.c b/drivers/net/can/dev/length.c
> index 5e7d481717ea..d695a3bee1ed 100644
> --- a/drivers/net/can/dev/length.c
> +++ b/drivers/net/can/dev/length.c
> @@ -27,15 +27,13 @@ static const u8 len2dlc[] = {
>         13, 13, 13, 13, 13, 13, 13, 13, /* 25 - 32 */
>         14, 14, 14, 14, 14, 14, 14, 14, /* 33 - 40 */
>         14, 14, 14, 14, 14, 14, 14, 14, /* 41 - 48 */
> -       15, 15, 15, 15, 15, 15, 15, 15, /* 49 - 56 */
> -       15, 15, 15, 15, 15, 15, 15, 15  /* 57 - 64 */
>  };
>
>  /* map the sanitized data length to an appropriate data length code */
>  u8 can_fd_len2dlc(u8 len)
>  {
> -       if (unlikely(len > 64))
> -               return 0xF;
> +       if (len > ARRAY_SIZE(len2dlc))

Sorry but I missed an of-by-one issue when I did my first
review. Don't know why but it popped to my eyes this morning when
casually reading the emails.

ARRAY_SIZE(len2dlc) is 49. If len is between 0 and 48, use the
array, if len is greater *or equal* return CANFD_MAX_DLC.

In short, replace > by >=:
+       if (len >= ARRAY_SIZE(len2dlc))

> +               return CANFD_MAX_DLC;
>
>         return len2dlc[len];
>  }

Yours sincerely,
Vincent