Re: [PATCH 8/8] firmware: vpd: fix confusion between memremap and iounmap

From: Guenter Roeck
Date: Wed May 24 2017 - 13:23:39 EST


On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> We should not be using iounmap to unmap memory mapped with memremap.
>
> This fixes following warnings generated by sparse in response to
> incorrect type annotations:
>
> CHECK drivers/firmware/google/vpd.c
> drivers/firmware/google/vpd.c:235:20: warning: incorrect type in argument 1 (different address spaces)
> drivers/firmware/google/vpd.c:235:20: expected void volatile [noderef] <asn:2>*addr
> drivers/firmware/google/vpd.c:235:20: got char *baseaddr
> drivers/firmware/google/vpd.c:246:28: warning: incorrect type in argument 1 (different address spaces)
> drivers/firmware/google/vpd.c:246:28: expected void volatile [noderef] <asn:2>*addr
> drivers/firmware/google/vpd.c:246:28: got char *baseaddr
> drivers/firmware/google/vpd.c:258:14: warning: incorrect type in assignment (different address spaces)
> drivers/firmware/google/vpd.c:258:14: expected struct vpd_cbmem [noderef] <asn:2>*temp
> drivers/firmware/google/vpd.c:258:14: got void *
>
> Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>

Reviewed-by: Guenter Roeck <groeck@xxxxxxxxxxxx>

> ---
> drivers/firmware/google/vpd.c | 39 ++++++++++++++++++++-------------------
> 1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
> index f4766bf6785a..b076210e38a2 100644
> --- a/drivers/firmware/google/vpd.c
> +++ b/drivers/firmware/google/vpd.c
> @@ -200,7 +200,7 @@ static int vpd_section_init(const char *name, struct vpd_section *sec,
> sec->raw_name = kasprintf(GFP_KERNEL, "%s_raw", name);
> if (!sec->raw_name) {
> err = -ENOMEM;
> - goto err_iounmap;
> + goto err_unmap;
> }
>
> sysfs_bin_attr_init(&sec->bin_attr);
> @@ -231,8 +231,8 @@ static int vpd_section_init(const char *name, struct vpd_section *sec,
> sysfs_remove_bin_file(vpd_kobj, &sec->bin_attr);
> err_free_raw_name:
> kfree(sec->raw_name);
> -err_iounmap:
> - iounmap(sec->baseaddr);
> +err_unmap:
> + memunmap(sec->baseaddr);
> return err;
> }
>
> @@ -243,7 +243,7 @@ static int vpd_section_destroy(struct vpd_section *sec)
> kobject_put(sec->kobj);
> sysfs_remove_bin_file(vpd_kobj, &sec->bin_attr);
> kfree(sec->raw_name);
> - iounmap(sec->baseaddr);
> + memunmap(sec->baseaddr);
> }
>
> return 0;
> @@ -251,38 +251,39 @@ static int vpd_section_destroy(struct vpd_section *sec)
>
> static int vpd_sections_init(phys_addr_t physaddr)
> {
> - struct vpd_cbmem __iomem *temp;
> - struct vpd_cbmem header;
> + struct vpd_cbmem *header;
> int ret = 0;
>
> - temp = memremap(physaddr, sizeof(struct vpd_cbmem), MEMREMAP_WB);
> - if (!temp)
> + header = memremap(physaddr, sizeof(struct vpd_cbmem), MEMREMAP_WB);
> + if (!header)
> return -ENOMEM;
>
> - memcpy_fromio(&header, temp, sizeof(struct vpd_cbmem));
> - iounmap(temp);
> -
> - if (header.magic != VPD_CBMEM_MAGIC)
> - return -ENODEV;
> + if (header->magic != VPD_CBMEM_MAGIC) {
> + ret = -ENODEV;
> + goto out;
> + }
>
> - if (header.ro_size) {
> + if (header->ro_size) {
> ret = vpd_section_init("ro", &ro_vpd,
> physaddr + sizeof(struct vpd_cbmem),
> - header.ro_size);
> + header->ro_size);
> if (ret)
> - return ret;
> + goto out;
> }
>
> - if (header.rw_size) {
> + if (header->rw_size) {
> ret = vpd_section_init("rw", &rw_vpd,
> physaddr + sizeof(struct vpd_cbmem) +
> - header.ro_size, header.rw_size);
> + header->ro_size,
> + header->rw_size);
> if (ret) {
> vpd_section_destroy(&ro_vpd);
> - return ret;
> + goto out;
> }
> }
>
> +out:
> + memunmap(header);
> return 0;
> }
>
> --
> 2.13.0.219.gdb65acc882-goog
>