Re: mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Colin, Stefan,

+linux-mtd

Thanks Colin for the report.

On Tue, 26 Jun 2018 16:18:29 +0100, Colin Ian King
<colin.king@canonical.com> wrote:

> Hi there,
> 
> Static analysis with CoverityScan reported a potential issue with the
> following commit:
> 
> commit 0f7b126ca91101d02d525f7cc880e8c71202a2b7
> Author: Stefan Agner <stefan@agner.ch>
> Date:   Sun Jun 24 23:27:25 2018 +0200
> 
>     mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
> 
> 
> in function tegra_nand_cmd it looks like there maybe potential to pass a
> negative value in size into memcpy():
> 
>         case NAND_OP_DATA_OUT_INSTR:
> 
> negative_return_fn: Function nand_subop_get_data_len(subop, op_id)
> returns a negative number.
> 
> var_assign: Assigning: unsigned variable size = nand_subop_get_data_len.
> 
>                 size = nand_subop_get_data_len(subop, op_id);
>                 offset = nand_subop_get_data_start_off(subop, op_id);

Stefan,

I thought a bit about this and I don't think the right place for such a
fix are the NAND controller drivers (marvell and vf610 have the same
issue). Both nand_subop_get_data/addr_len/start_off() are core helpers
and their result is predictable in a manner that only a bug in your
parsing function would trigger an error value. I think this is safe for
the four helpers to have WARN_ON() on the error conditions to catch
the developer's attention and just return (unsigned int) 0 in this case.

I will propose something soon.

Thanks,
Miquèl