2015-07-25 16:50:28

by Alexey Khoroshilov

Subject: [PATCH] mtd: gpio-addr-flash: add missing iounmap in probe/remove

There is no iounmap(state->map.virt) in gpio_flash_remove() and
in gpio_flash_probe() error handling code.

By the way the patch adds checks for error code of ioremap() and
mtd_device_parse_register().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <[email protected]>
---
drivers/mtd/maps/gpio-addr-flash.c | 35 +++++++++++++++++++++++++++--------
1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/maps/gpio-addr-flash.c b/drivers/mtd/maps/gpio-addr-flash.c
index 2fb346091af2..6b9e47d65f08 100644
--- a/drivers/mtd/maps/gpio-addr-flash.c
+++ b/drivers/mtd/maps/gpio-addr-flash.c
@@ -208,6 +208,7 @@ static int gpio_flash_probe(struct platform_device *pdev)
struct resource *memory;
struct resource *gpios;
struct async_state *state;
+ int ret;

pdata = dev_get_platdata(&pdev->dev);
memory = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -242,6 +243,11 @@ static int gpio_flash_probe(struct platform_device *pdev)
state->map.phys = NO_XIP;
state->map.map_priv_1 = (unsigned long)state;

+ if (!state->map.virt) {
+ ret = -ENODEV;
+ goto err_ioremap;
+ }
+
platform_set_drvdata(pdev, state);

i = 0;
@@ -251,8 +257,8 @@ static int gpio_flash_probe(struct platform_device *pdev)
state->gpio_addrs[i]);
while (i--)
gpio_free(state->gpio_addrs[i]);
- kfree(state);
- return -EBUSY;
+ ret = -EBUSY;
+ goto err_gpio;
}
gpio_direction_output(state->gpio_addrs[i], 0);
} while (++i < state->gpio_count);
@@ -261,28 +267,41 @@ static int gpio_flash_probe(struct platform_device *pdev)
state->map.bankwidth * 8);
state->mtd = do_map_probe(memory->name, &state->map);
if (!state->mtd) {
- for (i = 0; i < state->gpio_count; ++i)
- gpio_free(state->gpio_addrs[i]);
- kfree(state);
- return -ENXIO;
+ ret = -ENXIO;
+ goto err_mtdmap;
}


- mtd_device_parse_register(state->mtd, part_probe_types, NULL,
- pdata->parts, pdata->nr_parts);
+ ret = mtd_device_parse_register(state->mtd, part_probe_types, NULL,
+ pdata->parts, pdata->nr_parts);
+ if (ret)
+ goto err_mtdreg;

return 0;
+
+err_mtdreg:
+ map_destroy(state->mtd);
+err_mtdmap:
+ for (i = 0; i < state->gpio_count; ++i)
+ gpio_free(state->gpio_addrs[i]);
+err_gpio:
+ iounmap(state->map.virt);
+err_ioremap:
+ kfree(state);
+ return ret;
}

static int gpio_flash_remove(struct platform_device *pdev)
{
struct async_state *state = platform_get_drvdata(pdev);
size_t i = 0;
+
do {
gpio_free(state->gpio_addrs[i]);
} while (++i < state->gpio_count);
mtd_device_unregister(state->mtd);
map_destroy(state->mtd);
+ iounmap(state->map.virt);
kfree(state);
return 0;
}
--
1.9.1

2015-07-25 17:44:03

by Ezequiel Garcia

Subject: Re: [PATCH] mtd: gpio-addr-flash: add missing iounmap in probe/remove

Hi Alexey,

On 25 July 2015 at 13:49, Alexey Khoroshilov <[email protected]> wrote:
> There is no iounmap(state->map.virt) in gpio_flash_remove() and
> in gpio_flash_probe() error handling code.
>

Is there any reason why we can't just use managed variants? E.g.

diff --git a/drivers/mtd/maps/gpio-addr-flash.c
b/drivers/mtd/maps/gpio-addr-flash.c
index 2fb3460..bbaa92b 100644
--- a/drivers/mtd/maps/gpio-addr-flash.c
+++ b/drivers/mtd/maps/gpio-addr-flash.c
@@ -238,7 +238,8 @@ static int gpio_flash_probe(struct platform_device *pdev)
state->map.copy_to = gf_copy_to;
state->map.bankwidth = pdata->width;
state->map.size = state->win_size * (1 << state->gpio_count);
- state->map.virt = ioremap_nocache(memory->start, state->map.size);
+ state->map.virt = devm_ioremap_nocache(&pdev->dev, memory->start,
+ state->map.size);
state->map.phys = NO_XIP;
state->map.map_priv_1 = (unsigned long)state;

Notice the error checks are still needed though, but managed stuff would make
it simpler.

--
Ezequiel García, VanguardiaSur
http://www.vanguardiasur.com.ar