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

[Yicong Yang <yangyicong@hisilicon.com>]

On 2021/4/9 11:21, Tudor.Ambarus@microchip.com wrote:
> 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.
> 

yes. I cannot find a proper fix tag. so perhaps:

  Cc: stable@vger.kernel.org

>> 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!

> 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/