Re: [PATCH] mtd: rawnand: tegra: fix error handling of subop helpers

[Boris Brezillon <boris.brezillon@bootlin.com>]

On Sat, 14 Jul 2018 18:32:51 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> A report from Colin Ian King pointed a CoverityScan issue where error
> values on these helpers where not checked in the drivers. These
> helpers could error out only in case of a software bug in driver code,
> not because of a runtime/hardware error but in any cases it is safer
> to handle these errors properly.
> 
> Fix the Tegra NAND controller driver implementation by checking
> potential negative error values coming from these helpers.

Hm, ok. I thought you were opting for a return 0 + WARN_ON() approach,
what made you change your mind?

> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> 
> This patch should have been part of the following series:
> http://lists.infradead.org/pipermail/linux-mtd/2018-July/082654.html
> It will be squashed with original commit introducing this driver.
> 
> Thanks,
> Miquèl
> 
> drivers/mtd/nand/raw/tegra_nand.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c
> index 22d6a7f9ff80..59ead8f41718 100644
> --- a/drivers/mtd/nand/raw/tegra_nand.c
> +++ b/drivers/mtd/nand/raw/tegra_nand.c
> @@ -379,6 +379,9 @@ static int tegra_nand_cmd(struct nand_chip *chip,
>  		case NAND_OP_ADDR_INSTR:
>  			offset = nand_subop_get_addr_start_off(subop, op_id);
>  			naddrs = nand_subop_get_num_addr_cyc(subop, op_id);
> +			if (offset < 0 || naddrs < 0)
> +				return -EINVAL;
> +
>  			addrs = &instr->ctx.addr.addrs[offset];
>  
>  			cmd |= COMMAND_ALE | COMMAND_ALE_SIZE(naddrs);
> @@ -395,6 +398,8 @@ static int tegra_nand_cmd(struct nand_chip *chip,
>  		case NAND_OP_DATA_IN_INSTR:
>  			size = nand_subop_get_data_len(subop, op_id);
>  			offset = nand_subop_get_data_start_off(subop, op_id);
> +			if (size < 0 || offset < 0)
> +				return -EINVAL;
>  
>  			cmd |= COMMAND_TRANS_SIZE(size) | COMMAND_PIO |
>  				COMMAND_RX | COMMAND_A_VALID;
> @@ -405,6 +410,8 @@ static int tegra_nand_cmd(struct nand_chip *chip,
>  		case NAND_OP_DATA_OUT_INSTR:
>  			size = nand_subop_get_data_len(subop, op_id);
>  			offset = nand_subop_get_data_start_off(subop, op_id);
> +			if (size < 0 || offset < 0)
> +				return -EINVAL;
>  
>  			cmd |= COMMAND_TRANS_SIZE(size) | COMMAND_PIO |
>  				COMMAND_TX | COMMAND_A_VALID;