Re: [RFC PATCH 2/2] mmc: renesas_sdhi: prevent overflow for max_req_size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Wolfram,

On Thu, Mar 14, 2019 at 11:35 PM Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
> max_req_size is calculated by 'max_blk_size * max_blk_count' in the TMIO
> core. So, specifying U32_MAX as max_blk_count will overflow this
> calculation. It will cause no harm in practice because the immense high
> number will overflow into another immense high number. However, it is
> not good coding practice, so calculate max_blk_count so that
> max_req_size will fit into unsigned int on ARM32/64.
>
> Thanks to the Renesas BSP team for the bug report!
>
> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>

Thanks for your patch!

> --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> @@ -95,8 +95,8 @@ static const struct renesas_sdhi_of_data of_rza2_compatible = {
>         .scc_offset     = 0 - 0x1000,
>         .taps           = rcar_gen3_scc_taps,
>         .taps_num       = ARRAY_SIZE(rcar_gen3_scc_taps),
> -       /* DMAC can handle 0xffffffff blk count but only 1 segment */
> -       .max_blk_count  = 0xffffffff,
> +       /* DMAC can handle 32bit blk count but only 1 segment */
> +       .max_blk_count  = UINT_MAX / TMIO_MAX_BLK_SIZE,

I have mixed feelings about this change:
  1. You're lying about the actual maximum (yes, there's a comment that
     mentions the real limit),
  2. This fixes the problem in this single (set of) drivers only, while about
     every other driver (not the mmc core) calculates
     "max_blk_size * max_blk_count", too.
  3. Some drivers use different limits (e.g. 2048, 4095, or 4096), so
     eventually having a common upper limit is not easy.

BTW, drivers/mmc/host/usdhi6rol0.c does it the other way around:

    mmc->max_blk_count = mmc->max_req_size / mmc->max_blk_size;

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux