| Message ID | 5d9b08afdad2fbc65bac48d8ae22f4925bb80512.1520592440.git.arvind.yadav.cs@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | mtd: use put_device() if device_register fail | expand |
On Fri, 9 Mar 2018 16:20:49 +0530 Arvind Yadav <arvind.yadav.cs@gmail.com> wrote: > if device_register() returned an error! Always use put_device() > to give up the reference initialized. > > Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com> > --- > drivers/mtd/ubi/vmt.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c > index 3fd8d7f..db85b68 100644 > --- a/drivers/mtd/ubi/vmt.c > +++ b/drivers/mtd/ubi/vmt.c > @@ -609,6 +609,7 @@ int ubi_add_volume(struct ubi_device *ubi, struct ubi_volume *vol) > return err; > > out_cdev: > + put_device(&vol->dev); > cdev_del(&vol->cdev); use-after-free bug here: put_device() has freed the vol obj, and you're dereferencing the pointer just after that. > return err; > }
Am Mittwoch, 14. März 2018, 19:56:52 CET schrieb Boris Brezillon: > On Fri, 9 Mar 2018 16:20:49 +0530 > > Arvind Yadav <arvind.yadav.cs@gmail.com> wrote: > > if device_register() returned an error! Always use put_device() > > to give up the reference initialized. > > > > Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com> > > --- > > > > drivers/mtd/ubi/vmt.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c > > index 3fd8d7f..db85b68 100644 > > --- a/drivers/mtd/ubi/vmt.c > > +++ b/drivers/mtd/ubi/vmt.c > > @@ -609,6 +609,7 @@ int ubi_add_volume(struct ubi_device *ubi, struct > > ubi_volume *vol)> > > return err; > > > > out_cdev: > > + put_device(&vol->dev); > > > > cdev_del(&vol->cdev); > > use-after-free bug here: put_device() has freed the vol obj, and you're > dereferencing the pointer just after that. eeek, thanks for looking at more context. Arvind, while you are right that put_device() is missing, please double check that freeing the devices is also correct. Thanks, //richard
On Thursday 15 March 2018 12:55 AM, Richard Weinberger wrote: > Am Mittwoch, 14. März 2018, 19:56:52 CET schrieb Boris Brezillon: >> On Fri, 9 Mar 2018 16:20:49 +0530 >> >> Arvind Yadav <arvind.yadav.cs@gmail.com> wrote: >>> if device_register() returned an error! Always use put_device() >>> to give up the reference initialized. >>> >>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com> >>> --- >>> >>> drivers/mtd/ubi/vmt.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c >>> index 3fd8d7f..db85b68 100644 >>> --- a/drivers/mtd/ubi/vmt.c >>> +++ b/drivers/mtd/ubi/vmt.c >>> @@ -609,6 +609,7 @@ int ubi_add_volume(struct ubi_device *ubi, struct >>> ubi_volume *vol)> >>> return err; >>> >>> out_cdev: >>> + put_device(&vol->dev); >>> >>> cdev_del(&vol->cdev); >> use-after-free bug here: put_device() has freed the vol obj, and you're >> dereferencing the pointer just after that. Thanks Boris, to point out this error. > eeek, thanks for looking at more context. > Arvind, while you are right that put_device() is missing, please double check > that freeing the devices is also correct. > > Thanks, > //richard Sorry for that. I will take care of this. ~arvind
diff --git a/drivers/mtd/ubi/vmt.c b/drivers/mtd/ubi/vmt.c index 3fd8d7f..db85b68 100644 --- a/drivers/mtd/ubi/vmt.c +++ b/drivers/mtd/ubi/vmt.c @@ -609,6 +609,7 @@ int ubi_add_volume(struct ubi_device *ubi, struct ubi_volume *vol) return err; out_cdev: + put_device(&vol->dev); cdev_del(&vol->cdev); return err; }
if device_register() returned an error! Always use put_device() to give up the reference initialized. Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com> --- drivers/mtd/ubi/vmt.c | 1 + 1 file changed, 1 insertion(+)