SPI flash based MTD devs can be registered/unregistered at any time through the sf probe command or the spi_flash_free() function.
This commit does not try to fix the root cause as it would probably require rewriting most of the code and have an mtd_info object instance per spi_flash object (not to mention that the the spi-flash layer is likely to be replaced by a spi-nor layer ported from Linux).
Instead, we try to be as safe as can be by checking the code returned by del_mtd_device() and complain loudly when there's nothing we can do about the deregistration failure. When that happens we also reset sf_mtd_info.priv to NULL, and check for NULL pointer in the mtd hooks so that -ENODEV is returned instead of hitting a NULL pointer dereference exception when the MTD instance is later accessed by a user.
Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com Tested-by: Heiko Schocher hs@denx.de --- Changes in v4: - Add T-b tags
Changes in v3: - New patch --- drivers/mtd/spi/sf_mtd.c | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c index aabbc3589435..68c36002bee2 100644 --- a/drivers/mtd/spi/sf_mtd.c +++ b/drivers/mtd/spi/sf_mtd.c @@ -18,6 +18,9 @@ static int spi_flash_mtd_erase(struct mtd_info *mtd, struct erase_info *instr) struct spi_flash *flash = mtd->priv; int err;
+ if (!flash) + return -ENODEV; + instr->state = MTD_ERASING;
err = spi_flash_erase(flash, instr->addr, instr->len); @@ -39,6 +42,9 @@ static int spi_flash_mtd_read(struct mtd_info *mtd, loff_t from, size_t len, struct spi_flash *flash = mtd->priv; int err;
+ if (!flash) + return -ENODEV; + err = spi_flash_read(flash, from, len, buf); if (!err) *retlen = len; @@ -52,6 +58,9 @@ static int spi_flash_mtd_write(struct mtd_info *mtd, loff_t to, size_t len, struct spi_flash *flash = mtd->priv; int err;
+ if (!flash) + return -ENODEV; + err = spi_flash_write(flash, to, len, buf); if (!err) *retlen = len; @@ -76,8 +85,13 @@ int spi_flash_mtd_register(struct spi_flash *flash) { int ret;
- if (sf_mtd_registered) - del_mtd_device(&sf_mtd_info); + if (sf_mtd_registered) { + ret = del_mtd_device(&sf_mtd_info); + if (ret) + return ret; + + sf_mtd_registered = false; + }
sf_mtd_registered = false; memset(&sf_mtd_info, 0, sizeof(sf_mtd_info)); @@ -110,5 +124,24 @@ int spi_flash_mtd_register(struct spi_flash *flash)
void spi_flash_mtd_unregister(void) { - del_mtd_device(&sf_mtd_info); + int ret; + + if (!sf_mtd_registered) + return; + + ret = del_mtd_device(&sf_mtd_info); + if (!ret) { + sf_mtd_registered = false; + return; + } + + /* + * Setting mtd->priv to NULL is the best we can do. Thanks to that, + * the MTD layer can still call mtd hooks without risking a + * use-after-free bug. Still, things should be fixed to prevent the + * spi_flash object from being destroyed when del_mtd_device() fails. + */ + sf_mtd_info.priv = NULL; + printf("Failed to unregister MTD %s and the spi_flash object is going away: you're in deep trouble!", + sf_mtd_info.name); }