Re: [PATCH v4 4/5] mtd: rawnand: meson: check buffer length

Miquel Raynal
2023-05-22T15:43:52Z

Hi Arseniy,

AVKrasnov@sberdevices.ru wrote on Mon, 15 May 2023 12:44:38 +0300:

> This NAND controller has limited buffer length, so check it before
> command execution to avoid length trim. Also check MTD write size on
> chip attach.
>
> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
> ---
> drivers/mtd/nand/raw/meson_nand.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
> index a31106c943d7..dc0d7160520b 100644
> --- a/drivers/mtd/nand/raw/meson_nand.c
> +++ b/drivers/mtd/nand/raw/meson_nand.c
> @@ -111,6 +111,8 @@
> #define NFC_USER_BYTES 2
> #define NFC_OOB_PER_ECC(nand) ((nand)->ecc.bytes + NFC_USER_BYTES)
>
> +#define NFC_CMD_RAW_LEN GENMASK(13, 0)
> +
> struct meson_nfc_nand_chip {
> struct list_head node;
> struct nand_chip nand;
> @@ -284,7 +286,7 @@ static void meson_nfc_cmd_access(struct nand_chip *nand, int raw, bool dir,
>
> if (raw) {
> len = mtd->writesize + mtd->oobsize;
> - cmd = (len & GENMASK(13, 0)) | scrambler | DMA_DIR(dir);
> + cmd = len | scrambler | DMA_DIR(dir);
> writel(cmd, nfc->reg_base + NFC_REG_CMD);
> return;
> }
> @@ -562,6 +564,9 @@ static int meson_nfc_read_buf(struct nand_chip *nand, u8 *buf, int len)
> u32 cmd;
> u8 *info;
>
> + if (len > NFC_CMD_RAW_LEN)
> + return -EINVAL;
> +
> info = kzalloc(PER_INFO_BYTE, GFP_KERNEL);
> if (!info)
> return -ENOMEM;
> @@ -571,7 +576,7 @@ static int meson_nfc_read_buf(struct nand_chip *nand, u8 *buf, int len)
> if (ret)
> goto out;
>
> - cmd = NFC_CMD_N2M | (len & GENMASK(13, 0));
> + cmd = NFC_CMD_N2M | len;
> writel(cmd, nfc->reg_base + NFC_REG_CMD);
>
> meson_nfc_drain_cmd(nfc);
> @@ -590,12 +595,15 @@ static int meson_nfc_write_buf(struct nand_chip *nand, u8 *buf, int len)
> int ret = 0;
> u32 cmd;
>
> + if (len > NFC_CMD_RAW_LEN)
> + return -EINVAL;

I think this helper is related to exec_op, if yes, you should probably
ask an early check in the exec_op implementation (perhaps as part of a
dedicated helper).

Same above.

Otherwise looks good.

> +
> ret = meson_nfc_dma_buffer_setup(nand, buf, len, NULL,
> 0, DMA_TO_DEVICE);
> if (ret)
> return ret;
>
> - cmd = NFC_CMD_M2N | (len & GENMASK(13, 0));
> + cmd = NFC_CMD_M2N | len;
> writel(cmd, nfc->reg_base + NFC_REG_CMD);
>
> meson_nfc_drain_cmd(nfc);
> @@ -1328,6 +1336,7 @@ static int meson_nand_attach_chip(struct nand_chip *nand)
> struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
> struct mtd_info *mtd = nand_to_mtd(nand);
> int nsectors = mtd->writesize / 1024;
> + int raw_writesize;
> int ret;
>
> if (!mtd->name) {
> @@ -1339,6 +1348,13 @@ static int meson_nand_attach_chip(struct nand_chip *nand)
> return -ENOMEM;
> }
>
> + raw_writesize = mtd->writesize + mtd->oobsize;
> + if (raw_writesize > NFC_CMD_RAW_LEN) {
> + dev_err(nfc->dev, "too big write size in raw mode: %d > %ld\n",
> + raw_writesize, NFC_CMD_RAW_LEN);
> + return -EINVAL;
> + }
> +
> if (nand->bbt_options & NAND_BBT_USE_FLASH)
> nand->bbt_options |= NAND_BBT_NO_OOB;
>

Thanks,
Miquèl