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

[<Tudor.Ambarus@microchip.com>]

On 4/9/21 7:03 AM, Yicong Yang wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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:

Fixes: b199489d37b2 ("mtd: spi-nor: add the framework for SPI NOR")
should be fine.

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