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

[Yicong Yang]

On 2021/4/9 12:13, Tudor.Ambarus at microchip.com wrote:
> 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 at 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 at 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.
> 

it does make sense. please add that and the Cc stable when applying. thanks.

>>
>>   Cc: stable at vger.kernel.org
>>
>>>> Signed-off-by: Xiang Chen <chenxiang66 at hisilicon.com>
>>>> Signed-off-by: Yicong Yang <yangyicong at hisilicon.com>
>>>
>>> Tested-by: Tudor Ambarus <tudor.ambarus at 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
>>>>
>>>
>>
>