| Message ID | 5-v1-7dedf20b2b75+4f785-vfio2_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Make vfio_mdev type safe | expand |
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
Shouldn't this be at the beginning of the series, though?
> From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Wednesday, March 24, 2021 1:55 AM > > There is a small race where the parent is NULL even though the kobj has > already been made visible in sysfs. > > For instance the attribute_group is made visible in sysfs_create_files() > and the mdev_type_attr_show() does: > > ret = attr->show(kobj, type->parent->dev, buf); > > Which will crash on NULL parent. Move the parent setup to before the type > pointer leaves the stack frame. > > Fixes: 7b96953bc640 ("vfio: Mediated device Core driver") > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> > --- > drivers/vfio/mdev/mdev_sysfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/vfio/mdev/mdev_sysfs.c > b/drivers/vfio/mdev/mdev_sysfs.c > index 321b4d13ead7b8..5219cdd6dbbc49 100644 > --- a/drivers/vfio/mdev/mdev_sysfs.c > +++ b/drivers/vfio/mdev/mdev_sysfs.c > @@ -105,6 +105,7 @@ static struct mdev_type > *add_mdev_supported_type(struct mdev_parent *parent, > return ERR_PTR(-ENOMEM); > > type->kobj.kset = parent->mdev_types_kset; > + type->parent = parent; > > ret = kobject_init_and_add(&type->kobj, &mdev_type_ktype, NULL, > "%s-%s", dev_driver_string(parent->dev), > @@ -132,7 +133,6 @@ static struct mdev_type > *add_mdev_supported_type(struct mdev_parent *parent, > } > > type->group = group; > - type->parent = parent; > return type; > > attrs_failed: > -- > 2.31.0
On 3/23/2021 7:55 PM, Jason Gunthorpe wrote: > There is a small race where the parent is NULL even though the kobj has > already been made visible in sysfs. > > For instance the attribute_group is made visible in sysfs_create_files() > and the mdev_type_attr_show() does: > > ret = attr->show(kobj, type->parent->dev, buf); > > Which will crash on NULL parent. Move the parent setup to before the type > pointer leaves the stack frame. > > Fixes: 7b96953bc640 ("vfio: Mediated device Core driver") > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- Looks good, Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
On Tue, 23 Mar 2021 14:55:22 -0300 Jason Gunthorpe <jgg@nvidia.com> wrote: > There is a small race where the parent is NULL even though the kobj has > already been made visible in sysfs. > > For instance the attribute_group is made visible in sysfs_create_files() > and the mdev_type_attr_show() does: > > ret = attr->show(kobj, type->parent->dev, buf); > > Which will crash on NULL parent. Move the parent setup to before the type > pointer leaves the stack frame. > > Fixes: 7b96953bc640 ("vfio: Mediated device Core driver") > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/vfio/mdev/mdev_sysfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On Tue, Mar 23, 2021 at 08:15:58PM +0100, Christoph Hellwig wrote: > Looks good, > > Reviewed-by: Christoph Hellwig <hch@lst.de> > > Shouldn't this be at the beginning of the series, though? Yes, done Jason
diff --git a/drivers/vfio/mdev/mdev_sysfs.c b/drivers/vfio/mdev/mdev_sysfs.c index 321b4d13ead7b8..5219cdd6dbbc49 100644 --- a/drivers/vfio/mdev/mdev_sysfs.c +++ b/drivers/vfio/mdev/mdev_sysfs.c @@ -105,6 +105,7 @@ static struct mdev_type *add_mdev_supported_type(struct mdev_parent *parent, return ERR_PTR(-ENOMEM); type->kobj.kset = parent->mdev_types_kset; + type->parent = parent; ret = kobject_init_and_add(&type->kobj, &mdev_type_ktype, NULL, "%s-%s", dev_driver_string(parent->dev), @@ -132,7 +133,6 @@ static struct mdev_type *add_mdev_supported_type(struct mdev_parent *parent, } type->group = group; - type->parent = parent; return type; attrs_failed:
There is a small race where the parent is NULL even though the kobj has already been made visible in sysfs. For instance the attribute_group is made visible in sysfs_create_files() and the mdev_type_attr_show() does: ret = attr->show(kobj, type->parent->dev, buf); Which will crash on NULL parent. Move the parent setup to before the type pointer leaves the stack frame. Fixes: 7b96953bc640 ("vfio: Mediated device Core driver") Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/vfio/mdev/mdev_sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)