| Message ID | 1446137443-5387-1-git-send-email-james.morse@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi James, Thanks for this fix. Acked-by: Baptiste Reynal <b.reynal@virtualopensystems.com> Tested-by: Baptiste Reynal <b.reynal@virtualopensystems.com> On Thu, Oct 29, 2015 at 5:50 PM, James Morse <james.morse@arm.com> wrote: > vfio_platform_{read,write}_mmio() call ioremap_nocache() to map > a region of io memory, which they store in struct vfio_platform_region to > be eventually re-used, or unmapped by vfio_platform_regions_cleanup(). > > These functions receive a copy of their struct vfio_platform_region > argument on the stack - so these mapped areas are always allocated, and > always leaked. > > Pass this argument as a pointer instead. > > Fixes: 6e3f26456009 "vfio/platform: read and write support for the device fd" > Signed-off-by: James Morse <james.morse@arm.com> > --- > drivers/vfio/platform/vfio_platform_common.c | 36 ++++++++++++++-------------- > 1 file changed, 18 insertions(+), 18 deletions(-) > > diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c > index f3b6299..ccf5da5 100644 > --- a/drivers/vfio/platform/vfio_platform_common.c > +++ b/drivers/vfio/platform/vfio_platform_common.c > @@ -308,17 +308,17 @@ static long vfio_platform_ioctl(void *device_data, > return -ENOTTY; > } > > -static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, > +static ssize_t vfio_platform_read_mmio(struct vfio_platform_region *reg, > char __user *buf, size_t count, > loff_t off) > { > unsigned int done = 0; > > - if (!reg.ioaddr) { > - reg.ioaddr = > - ioremap_nocache(reg.addr, reg.size); > + if (!reg->ioaddr) { > + reg->ioaddr = > + ioremap_nocache(reg->addr, reg->size); > > - if (!reg.ioaddr) > + if (!reg->ioaddr) > return -ENOMEM; > } > > @@ -328,7 +328,7 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, > if (count >= 4 && !(off % 4)) { > u32 val; > > - val = ioread32(reg.ioaddr + off); > + val = ioread32(reg->ioaddr + off); > if (copy_to_user(buf, &val, 4)) > goto err; > > @@ -336,7 +336,7 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, > } else if (count >= 2 && !(off % 2)) { > u16 val; > > - val = ioread16(reg.ioaddr + off); > + val = ioread16(reg->ioaddr + off); > if (copy_to_user(buf, &val, 2)) > goto err; > > @@ -344,7 +344,7 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, > } else { > u8 val; > > - val = ioread8(reg.ioaddr + off); > + val = ioread8(reg->ioaddr + off); > if (copy_to_user(buf, &val, 1)) > goto err; > > @@ -377,7 +377,7 @@ static ssize_t vfio_platform_read(void *device_data, char __user *buf, > return -EINVAL; > > if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO) > - return vfio_platform_read_mmio(vdev->regions[index], > + return vfio_platform_read_mmio(&vdev->regions[index], > buf, count, off); > else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO) > return -EINVAL; /* not implemented */ > @@ -385,17 +385,17 @@ static ssize_t vfio_platform_read(void *device_data, char __user *buf, > return -EINVAL; > } > > -static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, > +static ssize_t vfio_platform_write_mmio(struct vfio_platform_region *reg, > const char __user *buf, size_t count, > loff_t off) > { > unsigned int done = 0; > > - if (!reg.ioaddr) { > - reg.ioaddr = > - ioremap_nocache(reg.addr, reg.size); > + if (!reg->ioaddr) { > + reg->ioaddr = > + ioremap_nocache(reg->addr, reg->size); > > - if (!reg.ioaddr) > + if (!reg->ioaddr) > return -ENOMEM; > } > > @@ -407,7 +407,7 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, > > if (copy_from_user(&val, buf, 4)) > goto err; > - iowrite32(val, reg.ioaddr + off); > + iowrite32(val, reg->ioaddr + off); > > filled = 4; > } else if (count >= 2 && !(off % 2)) { > @@ -415,7 +415,7 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, > > if (copy_from_user(&val, buf, 2)) > goto err; > - iowrite16(val, reg.ioaddr + off); > + iowrite16(val, reg->ioaddr + off); > > filled = 2; > } else { > @@ -423,7 +423,7 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, > > if (copy_from_user(&val, buf, 1)) > goto err; > - iowrite8(val, reg.ioaddr + off); > + iowrite8(val, reg->ioaddr + off); > > filled = 1; > } > @@ -453,7 +453,7 @@ static ssize_t vfio_platform_write(void *device_data, const char __user *buf, > return -EINVAL; > > if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO) > - return vfio_platform_write_mmio(vdev->regions[index], > + return vfio_platform_write_mmio(&vdev->regions[index], > buf, count, off); > else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO) > return -EINVAL; /* not implemented */ > -- > 2.6.1 > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 10/30/2015 09:51 AM, Baptiste Reynal wrote: > Hi James, > > Thanks for this fix. > > Acked-by: Baptiste Reynal <b.reynal@virtualopensystems.com> > Tested-by: Baptiste Reynal <b.reynal@virtualopensystems.com> > > On Thu, Oct 29, 2015 at 5:50 PM, James Morse <james.morse@arm.com> wrote: >> vfio_platform_{read,write}_mmio() call ioremap_nocache() to map >> a region of io memory, which they store in struct vfio_platform_region to >> be eventually re-used, or unmapped by vfio_platform_regions_cleanup(). >> >> These functions receive a copy of their struct vfio_platform_region >> argument on the stack - so these mapped areas are always allocated, and >> always leaked. I just noticed I have a leak in reset modules too. I am going to correct this. Thanks Eric >> >> Pass this argument as a pointer instead. >> >> Fixes: 6e3f26456009 "vfio/platform: read and write support for the device fd" >> Signed-off-by: James Morse <james.morse@arm.com> >> --- >> drivers/vfio/platform/vfio_platform_common.c | 36 ++++++++++++++-------------- >> 1 file changed, 18 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c >> index f3b6299..ccf5da5 100644 >> --- a/drivers/vfio/platform/vfio_platform_common.c >> +++ b/drivers/vfio/platform/vfio_platform_common.c >> @@ -308,17 +308,17 @@ static long vfio_platform_ioctl(void *device_data, >> return -ENOTTY; >> } >> >> -static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, >> +static ssize_t vfio_platform_read_mmio(struct vfio_platform_region *reg, >> char __user *buf, size_t count, >> loff_t off) >> { >> unsigned int done = 0; >> >> - if (!reg.ioaddr) { >> - reg.ioaddr = >> - ioremap_nocache(reg.addr, reg.size); >> + if (!reg->ioaddr) { >> + reg->ioaddr = >> + ioremap_nocache(reg->addr, reg->size); >> >> - if (!reg.ioaddr) >> + if (!reg->ioaddr) >> return -ENOMEM; >> } >> >> @@ -328,7 +328,7 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, >> if (count >= 4 && !(off % 4)) { >> u32 val; >> >> - val = ioread32(reg.ioaddr + off); >> + val = ioread32(reg->ioaddr + off); >> if (copy_to_user(buf, &val, 4)) >> goto err; >> >> @@ -336,7 +336,7 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, >> } else if (count >= 2 && !(off % 2)) { >> u16 val; >> >> - val = ioread16(reg.ioaddr + off); >> + val = ioread16(reg->ioaddr + off); >> if (copy_to_user(buf, &val, 2)) >> goto err; >> >> @@ -344,7 +344,7 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, >> } else { >> u8 val; >> >> - val = ioread8(reg.ioaddr + off); >> + val = ioread8(reg->ioaddr + off); >> if (copy_to_user(buf, &val, 1)) >> goto err; >> >> @@ -377,7 +377,7 @@ static ssize_t vfio_platform_read(void *device_data, char __user *buf, >> return -EINVAL; >> >> if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO) >> - return vfio_platform_read_mmio(vdev->regions[index], >> + return vfio_platform_read_mmio(&vdev->regions[index], >> buf, count, off); >> else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO) >> return -EINVAL; /* not implemented */ >> @@ -385,17 +385,17 @@ static ssize_t vfio_platform_read(void *device_data, char __user *buf, >> return -EINVAL; >> } >> >> -static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, >> +static ssize_t vfio_platform_write_mmio(struct vfio_platform_region *reg, >> const char __user *buf, size_t count, >> loff_t off) >> { >> unsigned int done = 0; >> >> - if (!reg.ioaddr) { >> - reg.ioaddr = >> - ioremap_nocache(reg.addr, reg.size); >> + if (!reg->ioaddr) { >> + reg->ioaddr = >> + ioremap_nocache(reg->addr, reg->size); >> >> - if (!reg.ioaddr) >> + if (!reg->ioaddr) >> return -ENOMEM; >> } >> >> @@ -407,7 +407,7 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, >> >> if (copy_from_user(&val, buf, 4)) >> goto err; >> - iowrite32(val, reg.ioaddr + off); >> + iowrite32(val, reg->ioaddr + off); >> >> filled = 4; >> } else if (count >= 2 && !(off % 2)) { >> @@ -415,7 +415,7 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, >> >> if (copy_from_user(&val, buf, 2)) >> goto err; >> - iowrite16(val, reg.ioaddr + off); >> + iowrite16(val, reg->ioaddr + off); >> >> filled = 2; >> } else { >> @@ -423,7 +423,7 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, >> >> if (copy_from_user(&val, buf, 1)) >> goto err; >> - iowrite8(val, reg.ioaddr + off); >> + iowrite8(val, reg->ioaddr + off); >> >> filled = 1; >> } >> @@ -453,7 +453,7 @@ static ssize_t vfio_platform_write(void *device_data, const char __user *buf, >> return -EINVAL; >> >> if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO) >> - return vfio_platform_write_mmio(vdev->regions[index], >> + return vfio_platform_write_mmio(&vdev->regions[index], >> buf, count, off); >> else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO) >> return -EINVAL; /* not implemented */ >> -- >> 2.6.1 >> > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c index f3b6299..ccf5da5 100644 --- a/drivers/vfio/platform/vfio_platform_common.c +++ b/drivers/vfio/platform/vfio_platform_common.c @@ -308,17 +308,17 @@ static long vfio_platform_ioctl(void *device_data, return -ENOTTY; } -static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, +static ssize_t vfio_platform_read_mmio(struct vfio_platform_region *reg, char __user *buf, size_t count, loff_t off) { unsigned int done = 0; - if (!reg.ioaddr) { - reg.ioaddr = - ioremap_nocache(reg.addr, reg.size); + if (!reg->ioaddr) { + reg->ioaddr = + ioremap_nocache(reg->addr, reg->size); - if (!reg.ioaddr) + if (!reg->ioaddr) return -ENOMEM; } @@ -328,7 +328,7 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, if (count >= 4 && !(off % 4)) { u32 val; - val = ioread32(reg.ioaddr + off); + val = ioread32(reg->ioaddr + off); if (copy_to_user(buf, &val, 4)) goto err; @@ -336,7 +336,7 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, } else if (count >= 2 && !(off % 2)) { u16 val; - val = ioread16(reg.ioaddr + off); + val = ioread16(reg->ioaddr + off); if (copy_to_user(buf, &val, 2)) goto err; @@ -344,7 +344,7 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, } else { u8 val; - val = ioread8(reg.ioaddr + off); + val = ioread8(reg->ioaddr + off); if (copy_to_user(buf, &val, 1)) goto err; @@ -377,7 +377,7 @@ static ssize_t vfio_platform_read(void *device_data, char __user *buf, return -EINVAL; if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO) - return vfio_platform_read_mmio(vdev->regions[index], + return vfio_platform_read_mmio(&vdev->regions[index], buf, count, off); else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO) return -EINVAL; /* not implemented */ @@ -385,17 +385,17 @@ static ssize_t vfio_platform_read(void *device_data, char __user *buf, return -EINVAL; } -static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, +static ssize_t vfio_platform_write_mmio(struct vfio_platform_region *reg, const char __user *buf, size_t count, loff_t off) { unsigned int done = 0; - if (!reg.ioaddr) { - reg.ioaddr = - ioremap_nocache(reg.addr, reg.size); + if (!reg->ioaddr) { + reg->ioaddr = + ioremap_nocache(reg->addr, reg->size); - if (!reg.ioaddr) + if (!reg->ioaddr) return -ENOMEM; } @@ -407,7 +407,7 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, if (copy_from_user(&val, buf, 4)) goto err; - iowrite32(val, reg.ioaddr + off); + iowrite32(val, reg->ioaddr + off); filled = 4; } else if (count >= 2 && !(off % 2)) { @@ -415,7 +415,7 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, if (copy_from_user(&val, buf, 2)) goto err; - iowrite16(val, reg.ioaddr + off); + iowrite16(val, reg->ioaddr + off); filled = 2; } else { @@ -423,7 +423,7 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, if (copy_from_user(&val, buf, 1)) goto err; - iowrite8(val, reg.ioaddr + off); + iowrite8(val, reg->ioaddr + off); filled = 1; } @@ -453,7 +453,7 @@ static ssize_t vfio_platform_write(void *device_data, const char __user *buf, return -EINVAL; if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO) - return vfio_platform_write_mmio(vdev->regions[index], + return vfio_platform_write_mmio(&vdev->regions[index], buf, count, off); else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO) return -EINVAL; /* not implemented */
vfio_platform_{read,write}_mmio() call ioremap_nocache() to map a region of io memory, which they store in struct vfio_platform_region to be eventually re-used, or unmapped by vfio_platform_regions_cleanup(). These functions receive a copy of their struct vfio_platform_region argument on the stack - so these mapped areas are always allocated, and always leaked. Pass this argument as a pointer instead. Fixes: 6e3f26456009 "vfio/platform: read and write support for the device fd" Signed-off-by: James Morse <james.morse@arm.com> --- drivers/vfio/platform/vfio_platform_common.c | 36 ++++++++++++++-------------- 1 file changed, 18 insertions(+), 18 deletions(-)