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
