Re: [PATCH] stm class: fix device leak in open error path
On Fri, Nov 18, 2016 at 01:55:58PM +0200, Alexander Shishkin wrote: > Johan Hovoldwrites: > > > Make sure to drop the reference taken by class_find_device() also on > > allocation errors in open(). > > > > Fixes: 7bd1d4093c2f ("stm class: Introduce an abstraction for...") > > Signed-off-by: Johan Hovold > > Good catch, thanks! I'm going to change it a bit to look like this > if you don't mind: > > diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c > index 51f81d64ca..cf6c9ea14c 100644 > --- a/drivers/hwtracing/stm/core.c > +++ b/drivers/hwtracing/stm/core.c > @@ -361,7 +361,7 @@ static int stm_char_open(struct inode *inode, struct file > *file) > struct stm_file *stmf; > struct device *dev; > unsigned int major = imajor(inode); > - int err = -ENODEV; > + int err = -ENOMEM; > > dev = class_find_device(_class, NULL, , major_match); > if (!dev) > @@ -369,8 +369,9 @@ static int stm_char_open(struct inode *inode, struct file > *file) > > stmf = kzalloc(sizeof(*stmf), GFP_KERNEL); > if (!stmf) > - return -ENOMEM; > + goto err_put_device; > > + err = -ENODEV; > stm_output_init(>output); > stmf->stm = to_stm_device(dev); Sure. I find it more readable to set the errno in the error path, but I don't mind. Thanks, Johan
Re: [PATCH] stm class: fix device leak in open error path
On Fri, Nov 18, 2016 at 01:55:58PM +0200, Alexander Shishkin wrote: > Johan Hovold writes: > > > Make sure to drop the reference taken by class_find_device() also on > > allocation errors in open(). > > > > Fixes: 7bd1d4093c2f ("stm class: Introduce an abstraction for...") > > Signed-off-by: Johan Hovold > > Good catch, thanks! I'm going to change it a bit to look like this > if you don't mind: > > diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c > index 51f81d64ca..cf6c9ea14c 100644 > --- a/drivers/hwtracing/stm/core.c > +++ b/drivers/hwtracing/stm/core.c > @@ -361,7 +361,7 @@ static int stm_char_open(struct inode *inode, struct file > *file) > struct stm_file *stmf; > struct device *dev; > unsigned int major = imajor(inode); > - int err = -ENODEV; > + int err = -ENOMEM; > > dev = class_find_device(_class, NULL, , major_match); > if (!dev) > @@ -369,8 +369,9 @@ static int stm_char_open(struct inode *inode, struct file > *file) > > stmf = kzalloc(sizeof(*stmf), GFP_KERNEL); > if (!stmf) > - return -ENOMEM; > + goto err_put_device; > > + err = -ENODEV; > stm_output_init(>output); > stmf->stm = to_stm_device(dev); Sure. I find it more readable to set the errno in the error path, but I don't mind. Thanks, Johan
Re: [PATCH] stm class: fix device leak in open error path
Johan Hovoldwrites: > Make sure to drop the reference taken by class_find_device() also on > allocation errors in open(). > > Fixes: 7bd1d4093c2f ("stm class: Introduce an abstraction for...") > Signed-off-by: Johan Hovold Good catch, thanks! I'm going to change it a bit to look like this if you don't mind: diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c index 51f81d64ca..cf6c9ea14c 100644 --- a/drivers/hwtracing/stm/core.c +++ b/drivers/hwtracing/stm/core.c @@ -361,7 +361,7 @@ static int stm_char_open(struct inode *inode, struct file *file) struct stm_file *stmf; struct device *dev; unsigned int major = imajor(inode); - int err = -ENODEV; + int err = -ENOMEM; dev = class_find_device(_class, NULL, , major_match); if (!dev) @@ -369,8 +369,9 @@ static int stm_char_open(struct inode *inode, struct file *file) stmf = kzalloc(sizeof(*stmf), GFP_KERNEL); if (!stmf) - return -ENOMEM; + goto err_put_device; + err = -ENODEV; stm_output_init(>output); stmf->stm = to_stm_device(dev); @@ -382,9 +383,10 @@ static int stm_char_open(struct inode *inode, struct file *file) return nonseekable_open(inode, file); err_free: + kfree(stmf); +err_put_device: /* matches class_find_device() above */ put_device(dev); - kfree(stmf); return err; } Regards, -- Alex
Re: [PATCH] stm class: fix device leak in open error path
Johan Hovold writes: > Make sure to drop the reference taken by class_find_device() also on > allocation errors in open(). > > Fixes: 7bd1d4093c2f ("stm class: Introduce an abstraction for...") > Signed-off-by: Johan Hovold Good catch, thanks! I'm going to change it a bit to look like this if you don't mind: diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c index 51f81d64ca..cf6c9ea14c 100644 --- a/drivers/hwtracing/stm/core.c +++ b/drivers/hwtracing/stm/core.c @@ -361,7 +361,7 @@ static int stm_char_open(struct inode *inode, struct file *file) struct stm_file *stmf; struct device *dev; unsigned int major = imajor(inode); - int err = -ENODEV; + int err = -ENOMEM; dev = class_find_device(_class, NULL, , major_match); if (!dev) @@ -369,8 +369,9 @@ static int stm_char_open(struct inode *inode, struct file *file) stmf = kzalloc(sizeof(*stmf), GFP_KERNEL); if (!stmf) - return -ENOMEM; + goto err_put_device; + err = -ENODEV; stm_output_init(>output); stmf->stm = to_stm_device(dev); @@ -382,9 +383,10 @@ static int stm_char_open(struct inode *inode, struct file *file) return nonseekable_open(inode, file); err_free: + kfree(stmf); +err_put_device: /* matches class_find_device() above */ put_device(dev); - kfree(stmf); return err; } Regards, -- Alex
Re: [PATCH] stm class: fix device leak in open error path
On Tue, Nov 01, 2016 at 11:44:54AM +0100, Johan Hovold wrote: > Make sure to drop the reference taken by class_find_device() also on > allocation errors in open(). > > Fixes: 7bd1d4093c2f ("stm class: Introduce an abstraction for...") > Signed-off-by: Johan Hovold> --- > drivers/hwtracing/stm/core.c | 9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c > index 51f81d64ca37..47811f5c9bc9 100644 > --- a/drivers/hwtracing/stm/core.c > +++ b/drivers/hwtracing/stm/core.c > @@ -368,8 +368,10 @@ static int stm_char_open(struct inode *inode, struct > file *file) > return -ENODEV; > > stmf = kzalloc(sizeof(*stmf), GFP_KERNEL); > - if (!stmf) > - return -ENOMEM; > + if (!stmf) { > + err = -ENOMEM; > + goto err_put_device; > + } > > stm_output_init(>output); > stmf->stm = to_stm_device(dev); > @@ -382,9 +384,10 @@ static int stm_char_open(struct inode *inode, struct > file *file) > return nonseekable_open(inode, file); > > err_free: > + kfree(stmf); > +err_put_device: > /* matches class_find_device() above */ > put_device(dev); > - kfree(stmf); > > return err; > } Any comments to this one, Alexander? Thanks, Johan
Re: [PATCH] stm class: fix device leak in open error path
On Tue, Nov 01, 2016 at 11:44:54AM +0100, Johan Hovold wrote: > Make sure to drop the reference taken by class_find_device() also on > allocation errors in open(). > > Fixes: 7bd1d4093c2f ("stm class: Introduce an abstraction for...") > Signed-off-by: Johan Hovold > --- > drivers/hwtracing/stm/core.c | 9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c > index 51f81d64ca37..47811f5c9bc9 100644 > --- a/drivers/hwtracing/stm/core.c > +++ b/drivers/hwtracing/stm/core.c > @@ -368,8 +368,10 @@ static int stm_char_open(struct inode *inode, struct > file *file) > return -ENODEV; > > stmf = kzalloc(sizeof(*stmf), GFP_KERNEL); > - if (!stmf) > - return -ENOMEM; > + if (!stmf) { > + err = -ENOMEM; > + goto err_put_device; > + } > > stm_output_init(>output); > stmf->stm = to_stm_device(dev); > @@ -382,9 +384,10 @@ static int stm_char_open(struct inode *inode, struct > file *file) > return nonseekable_open(inode, file); > > err_free: > + kfree(stmf); > +err_put_device: > /* matches class_find_device() above */ > put_device(dev); > - kfree(stmf); > > return err; > } Any comments to this one, Alexander? Thanks, Johan
[PATCH] stm class: fix device leak in open error path
Make sure to drop the reference taken by class_find_device() also on allocation errors in open(). Fixes: 7bd1d4093c2f ("stm class: Introduce an abstraction for...") Signed-off-by: Johan Hovold--- drivers/hwtracing/stm/core.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c index 51f81d64ca37..47811f5c9bc9 100644 --- a/drivers/hwtracing/stm/core.c +++ b/drivers/hwtracing/stm/core.c @@ -368,8 +368,10 @@ static int stm_char_open(struct inode *inode, struct file *file) return -ENODEV; stmf = kzalloc(sizeof(*stmf), GFP_KERNEL); - if (!stmf) - return -ENOMEM; + if (!stmf) { + err = -ENOMEM; + goto err_put_device; + } stm_output_init(>output); stmf->stm = to_stm_device(dev); @@ -382,9 +384,10 @@ static int stm_char_open(struct inode *inode, struct file *file) return nonseekable_open(inode, file); err_free: + kfree(stmf); +err_put_device: /* matches class_find_device() above */ put_device(dev); - kfree(stmf); return err; } -- 2.7.3
[PATCH] stm class: fix device leak in open error path
Make sure to drop the reference taken by class_find_device() also on allocation errors in open(). Fixes: 7bd1d4093c2f ("stm class: Introduce an abstraction for...") Signed-off-by: Johan Hovold --- drivers/hwtracing/stm/core.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c index 51f81d64ca37..47811f5c9bc9 100644 --- a/drivers/hwtracing/stm/core.c +++ b/drivers/hwtracing/stm/core.c @@ -368,8 +368,10 @@ static int stm_char_open(struct inode *inode, struct file *file) return -ENODEV; stmf = kzalloc(sizeof(*stmf), GFP_KERNEL); - if (!stmf) - return -ENOMEM; + if (!stmf) { + err = -ENOMEM; + goto err_put_device; + } stm_output_init(>output); stmf->stm = to_stm_device(dev); @@ -382,9 +384,10 @@ static int stm_char_open(struct inode *inode, struct file *file) return nonseekable_open(inode, file); err_free: + kfree(stmf); +err_put_device: /* matches class_find_device() above */ put_device(dev); - kfree(stmf); return err; } -- 2.7.3