Re: [Intel-gfx] [PATCH v2 02/14] vfio/mbochs: Fix missing error unwind in mbochs_probe()
On Tue, Jul 20 2021, Jason Gunthorpe wrote: > On Tue, Jul 20, 2021 at 04:01:27PM -0600, Alex Williamson wrote: >> Hmm, doesn't this suggest we need another atomic conversion? (untested) > > Sure why not, I can add this as another patch Yes, I think that should be another patch. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 02/14] vfio/mbochs: Fix missing error unwind in mbochs_probe()
On Tue, Jul 20 2021, Jason Gunthorpe wrote:
> Compared to mbochs_remove() two cases are missing from the
> vfio_register_group_dev() unwind. Add them in.
>
> Fixes: 681c1615f891 ("vfio/mbochs: Convert to use vfio_register_group_dev()")
> Reported-by: Cornelia Huck
> Signed-off-by: Jason Gunthorpe
> ---
> samples/vfio-mdev/mbochs.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
Reviewed-by: Cornelia Huck
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 02/14] vfio/mbochs: Fix missing error unwind in mbochs_probe()
On Tue, Jul 20, 2021 at 04:01:27PM -0600, Alex Williamson wrote:
> On Tue, 20 Jul 2021 14:42:48 -0300
> Jason Gunthorpe wrote:
>
> > Compared to mbochs_remove() two cases are missing from the
> > vfio_register_group_dev() unwind. Add them in.
> >
> > Fixes: 681c1615f891 ("vfio/mbochs: Convert to use
> > vfio_register_group_dev()")
> > Reported-by: Cornelia Huck
> > Signed-off-by: Jason Gunthorpe
> > samples/vfio-mdev/mbochs.c | 7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c
> > index e81b875b4d87b4..501845b08c0974 100644
> > +++ b/samples/vfio-mdev/mbochs.c
> > @@ -553,11 +553,14 @@ static int mbochs_probe(struct mdev_device *mdev)
> >
> > ret = vfio_register_group_dev(_state->vdev);
> > if (ret)
> > - goto err_mem;
> > + goto err_bytes;
> > dev_set_drvdata(>dev, mdev_state);
> > return 0;
> >
> > +err_bytes:
> > + mbochs_used_mbytes -= mdev_state->type->mbytes;
> > err_mem:
> > + kfree(mdev_state->pages);
> > kfree(mdev_state->vconfig);
> > kfree(mdev_state);
> > return ret;
> > @@ -567,8 +570,8 @@ static void mbochs_remove(struct mdev_device *mdev)
> > {
> > struct mdev_state *mdev_state = dev_get_drvdata(>dev);
> >
> > - mbochs_used_mbytes -= mdev_state->type->mbytes;
> > vfio_unregister_group_dev(_state->vdev);
> > + mbochs_used_mbytes -= mdev_state->type->mbytes;
> > kfree(mdev_state->pages);
> > kfree(mdev_state->vconfig);
> > kfree(mdev_state);
>
> Hmm, doesn't this suggest we need another atomic conversion? (untested)
Sure why not, I can add this as another patch
> @@ -567,11 +573,11 @@ static void mbochs_remove(struct mdev_device *mdev)
> {
> struct mdev_state *mdev_state = dev_get_drvdata(>dev);
>
> - mbochs_used_mbytes -= mdev_state->type->mbytes;
> vfio_unregister_group_dev(_state->vdev);
> kfree(mdev_state->pages);
> kfree(mdev_state->vconfig);
> kfree(mdev_state);
> + atomic_add(mdev_state->type->mbytes, _avail_mbytes);
This should be up after the vfio_unregister_group_dev(), it is a use after free?
Jason
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 02/14] vfio/mbochs: Fix missing error unwind in mbochs_probe()
On Tue, 20 Jul 2021 19:49:55 -0300
Jason Gunthorpe wrote:
> On Tue, Jul 20, 2021 at 04:01:27PM -0600, Alex Williamson wrote:
> > On Tue, 20 Jul 2021 14:42:48 -0300
> > Jason Gunthorpe wrote:
> >
> > > Compared to mbochs_remove() two cases are missing from the
> > > vfio_register_group_dev() unwind. Add them in.
> > >
> > > Fixes: 681c1615f891 ("vfio/mbochs: Convert to use
> > > vfio_register_group_dev()")
> > > Reported-by: Cornelia Huck
> > > Signed-off-by: Jason Gunthorpe
> > > samples/vfio-mdev/mbochs.c | 7 +--
> > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c
> > > index e81b875b4d87b4..501845b08c0974 100644
> > > +++ b/samples/vfio-mdev/mbochs.c
> > > @@ -553,11 +553,14 @@ static int mbochs_probe(struct mdev_device *mdev)
> > >
> > > ret = vfio_register_group_dev(_state->vdev);
> > > if (ret)
> > > - goto err_mem;
> > > + goto err_bytes;
> > > dev_set_drvdata(>dev, mdev_state);
> > > return 0;
> > >
> > > +err_bytes:
> > > + mbochs_used_mbytes -= mdev_state->type->mbytes;
> > > err_mem:
> > > + kfree(mdev_state->pages);
> > > kfree(mdev_state->vconfig);
> > > kfree(mdev_state);
> > > return ret;
> > > @@ -567,8 +570,8 @@ static void mbochs_remove(struct mdev_device *mdev)
> > > {
> > > struct mdev_state *mdev_state = dev_get_drvdata(>dev);
> > >
> > > - mbochs_used_mbytes -= mdev_state->type->mbytes;
> > > vfio_unregister_group_dev(_state->vdev);
> > > + mbochs_used_mbytes -= mdev_state->type->mbytes;
> > > kfree(mdev_state->pages);
> > > kfree(mdev_state->vconfig);
> > > kfree(mdev_state);
> >
> > Hmm, doesn't this suggest we need another atomic conversion? (untested)
>
> Sure why not, I can add this as another patch
>
> > @@ -567,11 +573,11 @@ static void mbochs_remove(struct mdev_device *mdev)
> > {
> > struct mdev_state *mdev_state = dev_get_drvdata(>dev);
> >
> > - mbochs_used_mbytes -= mdev_state->type->mbytes;
> > vfio_unregister_group_dev(_state->vdev);
> > kfree(mdev_state->pages);
> > kfree(mdev_state->vconfig);
> > kfree(mdev_state);
> > + atomic_add(mdev_state->type->mbytes, _avail_mbytes);
>
> This should be up after the vfio_unregister_group_dev(), it is a use after
> free?
Oops, yep. That or get the mbochs_type so we can mirror the _probe
setup. Same on the _probe unwind, but we've already got type->mbytes
there. Thanks,
Alex
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 02/14] vfio/mbochs: Fix missing error unwind in mbochs_probe()
On Tue, 20 Jul 2021 14:42:48 -0300
Jason Gunthorpe wrote:
> Compared to mbochs_remove() two cases are missing from the
> vfio_register_group_dev() unwind. Add them in.
>
> Fixes: 681c1615f891 ("vfio/mbochs: Convert to use vfio_register_group_dev()")
> Reported-by: Cornelia Huck
> Signed-off-by: Jason Gunthorpe
> ---
> samples/vfio-mdev/mbochs.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c
> index e81b875b4d87b4..501845b08c0974 100644
> --- a/samples/vfio-mdev/mbochs.c
> +++ b/samples/vfio-mdev/mbochs.c
> @@ -553,11 +553,14 @@ static int mbochs_probe(struct mdev_device *mdev)
>
> ret = vfio_register_group_dev(_state->vdev);
> if (ret)
> - goto err_mem;
> + goto err_bytes;
> dev_set_drvdata(>dev, mdev_state);
> return 0;
>
> +err_bytes:
> + mbochs_used_mbytes -= mdev_state->type->mbytes;
> err_mem:
> + kfree(mdev_state->pages);
> kfree(mdev_state->vconfig);
> kfree(mdev_state);
> return ret;
> @@ -567,8 +570,8 @@ static void mbochs_remove(struct mdev_device *mdev)
> {
> struct mdev_state *mdev_state = dev_get_drvdata(>dev);
>
> - mbochs_used_mbytes -= mdev_state->type->mbytes;
> vfio_unregister_group_dev(_state->vdev);
> + mbochs_used_mbytes -= mdev_state->type->mbytes;
> kfree(mdev_state->pages);
> kfree(mdev_state->vconfig);
> kfree(mdev_state);
Hmm, doesn't this suggest we need another atomic conversion? (untested)
diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c
index e81b875b4d87..842819e29c6b 100644
--- a/samples/vfio-mdev/mbochs.c
+++ b/samples/vfio-mdev/mbochs.c
@@ -129,7 +129,7 @@ static dev_tmbochs_devt;
static struct class*mbochs_class;
static struct cdev mbochs_cdev;
static struct device mbochs_dev;
-static int mbochs_used_mbytes;
+static atomic_tmbochs_avail_mbytes;
static const struct vfio_device_ops mbochs_dev_ops;
struct vfio_region_info_ext {
@@ -511,14 +511,19 @@ static int mbochs_probe(struct mdev_device *mdev)
_types[mdev_get_type_group_id(mdev)];
struct device *dev = mdev_dev(mdev);
struct mdev_state *mdev_state;
+ int avail_mbytes = atomic_read(_avail_mbytes);
int ret = -ENOMEM;
- if (type->mbytes + mbochs_used_mbytes > max_mbytes)
- return -ENOMEM;
+ do {
+ if (avail_mbytes < type->mbytes)
+ return ret;
+ } while (!atomic_try_cmpxchg(_avail_mbytes, _mbytes,
+avail_mbytes - type->mbytes));
mdev_state = kzalloc(sizeof(struct mdev_state), GFP_KERNEL);
if (mdev_state == NULL)
- return -ENOMEM;
+ goto err_resv;
+
vfio_init_group_dev(_state->vdev, >dev, _dev_ops);
mdev_state->vconfig = kzalloc(MBOCHS_CONFIG_SPACE_SIZE, GFP_KERNEL);
@@ -549,8 +554,6 @@ static int mbochs_probe(struct mdev_device *mdev)
mbochs_create_config_space(mdev_state);
mbochs_reset(mdev_state);
- mbochs_used_mbytes += type->mbytes;
-
ret = vfio_register_group_dev(_state->vdev);
if (ret)
goto err_mem;
@@ -558,8 +561,11 @@ static int mbochs_probe(struct mdev_device *mdev)
return 0;
err_mem:
+ kfree(mdev_state->pages);
kfree(mdev_state->vconfig);
kfree(mdev_state);
+err_resv:
+ atomic_add(mdev_state->type->mbytes, _avail_mbytes);
return ret;
}
@@ -567,11 +573,11 @@ static void mbochs_remove(struct mdev_device *mdev)
{
struct mdev_state *mdev_state = dev_get_drvdata(>dev);
- mbochs_used_mbytes -= mdev_state->type->mbytes;
vfio_unregister_group_dev(_state->vdev);
kfree(mdev_state->pages);
kfree(mdev_state->vconfig);
kfree(mdev_state);
+ atomic_add(mdev_state->type->mbytes, _avail_mbytes);
}
static ssize_t mbochs_read(struct vfio_device *vdev, char __user *buf,
@@ -1351,7 +1357,7 @@ static ssize_t available_instances_show(struct mdev_type
*mtype,
{
const struct mbochs_type *type =
_types[mtype_get_type_group_id(mtype)];
- int count = (max_mbytes - mbochs_used_mbytes) / type->mbytes;
+ int count = atomic_read(_avail_mbytes) / type->mbytes;
return sprintf(buf, "%d\n", count);
}
@@ -1460,6 +1466,8 @@ static int __init mbochs_dev_init(void)
if (ret)
goto err_class;
+ atomic_set(_avail_mbytes, max_mbytes);
+
ret = mdev_register_device(_dev, _fops);
if (ret)
goto err_device;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 02/14] vfio/mbochs: Fix missing error unwind in mbochs_probe()
Compared to mbochs_remove() two cases are missing from the
vfio_register_group_dev() unwind. Add them in.
Fixes: 681c1615f891 ("vfio/mbochs: Convert to use vfio_register_group_dev()")
Reported-by: Cornelia Huck
Signed-off-by: Jason Gunthorpe
---
samples/vfio-mdev/mbochs.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c
index e81b875b4d87b4..501845b08c0974 100644
--- a/samples/vfio-mdev/mbochs.c
+++ b/samples/vfio-mdev/mbochs.c
@@ -553,11 +553,14 @@ static int mbochs_probe(struct mdev_device *mdev)
ret = vfio_register_group_dev(_state->vdev);
if (ret)
- goto err_mem;
+ goto err_bytes;
dev_set_drvdata(>dev, mdev_state);
return 0;
+err_bytes:
+ mbochs_used_mbytes -= mdev_state->type->mbytes;
err_mem:
+ kfree(mdev_state->pages);
kfree(mdev_state->vconfig);
kfree(mdev_state);
return ret;
@@ -567,8 +570,8 @@ static void mbochs_remove(struct mdev_device *mdev)
{
struct mdev_state *mdev_state = dev_get_drvdata(>dev);
- mbochs_used_mbytes -= mdev_state->type->mbytes;
vfio_unregister_group_dev(_state->vdev);
+ mbochs_used_mbytes -= mdev_state->type->mbytes;
kfree(mdev_state->pages);
kfree(mdev_state->vconfig);
kfree(mdev_state);
--
2.32.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
