Re: [PATCH RESEND] mtd:spi-nor: Fix an issue of releasing resources during read/write

[<Tudor.Ambarus@microchip.com>]

On 4/1/21 10:34 AM, Yicong Yang wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> From: Xiang Chen <chenxiang66@hisilicon.com>
> 
> If rmmod the driver during read or write, the driver will release the
> resources which are used during read or write, so it is possible to
> refer to NULL pointer.
> 
> Use the testcase "mtd_debug read /dev/mtd0 0xc00000 0x400000 dest_file &
> sleep 0.5;rmmod spi_hisi_sfc_v3xx.ko", the issue can be reproduced in
> hisi_sfc_v3xx driver.
> 
> To avoid the issue, fill the interface _get_device and _put_device of
> mtd_info to grab the reference to the spi controller driver module, so
> the request of rmmod the driver is rejected before read/write is finished.
> 

Do we care for a Fixes tag and a Cc to stable? I guess this bug is present
since the introduction of the driver.

> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>

Tested-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Will fix the subject of the commit when applying.

Thanks,
ta
> ---
>  drivers/mtd/spi-nor/core.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 0522304..72bc134 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3301,6 +3301,37 @@ static void spi_nor_resume(struct mtd_info *mtd)
>                 dev_err(dev, "resume() failed\n");
>  }
> 
> +static int spi_nor_get_device(struct mtd_info *mtd)
> +{
> +       struct mtd_info *master = mtd_get_master(mtd);
> +       struct spi_nor *nor = mtd_to_spi_nor(master);
> +       struct device *dev;
> +
> +       if (nor->spimem)
> +               dev = nor->spimem->spi->controller->dev.parent;
> +       else
> +               dev = nor->dev;
> +
> +       if (!try_module_get(dev->driver->owner))
> +               return -ENODEV;
> +
> +       return 0;
> +}
> +
> +static void spi_nor_put_device(struct mtd_info *mtd)
> +{
> +       struct mtd_info *master = mtd_get_master(mtd);
> +       struct spi_nor *nor = mtd_to_spi_nor(master);
> +       struct device *dev;
> +
> +       if (nor->spimem)
> +               dev = nor->spimem->spi->controller->dev.parent;
> +       else
> +               dev = nor->dev;
> +
> +       module_put(dev->driver->owner);
> +}
> +
>  void spi_nor_restore(struct spi_nor *nor)
>  {
>         /* restore the addressing mode */
> @@ -3495,6 +3526,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>         mtd->_read = spi_nor_read;
>         mtd->_suspend = spi_nor_suspend;
>         mtd->_resume = spi_nor_resume;
> +       mtd->_get_device = spi_nor_get_device;
> +       mtd->_put_device = spi_nor_put_device;
> 
>         if (nor->params->locking_ops) {
>                 mtd->_lock = spi_nor_lock;
> --
> 2.8.1
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/