| Message ID | 1495535911-29036-1-git-send-email-idryomov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 23, 2017 at 12:38:31PM +0200, Ilya Dryomov wrote: > nbd_config is allocated in nbd_alloc_config(), but never freed. > > Fixes: 5ea8d10802ec ("nbd: separate out the config information") > Signed-off-by: Ilya Dryomov <idryomov@gmail.com> > --- > drivers/block/nbd.c | 1 + > 1 file changed, 1 insertion(+) > Oops, thanks Ilya Reviewed-by: Josef Bacik <jbacik@fb.com> Josef
On 05/23/2017 08:14 AM, Josef Bacik wrote: > On Tue, May 23, 2017 at 12:38:31PM +0200, Ilya Dryomov wrote: >> nbd_config is allocated in nbd_alloc_config(), but never freed. >> >> Fixes: 5ea8d10802ec ("nbd: separate out the config information") >> Signed-off-by: Ilya Dryomov <idryomov@gmail.com> >> --- >> drivers/block/nbd.c | 1 + >> 1 file changed, 1 insertion(+) >> > > Oops, thanks Ilya > > Reviewed-by: Josef Bacik <jbacik@fb.com> Since config is referenced, why isn't this done in nbd_config_put() instead?
On Tue, May 23, 2017 at 4:16 PM, Jens Axboe <axboe@fb.com> wrote: > On 05/23/2017 08:14 AM, Josef Bacik wrote: >> On Tue, May 23, 2017 at 12:38:31PM +0200, Ilya Dryomov wrote: >>> nbd_config is allocated in nbd_alloc_config(), but never freed. >>> >>> Fixes: 5ea8d10802ec ("nbd: separate out the config information") >>> Signed-off-by: Ilya Dryomov <idryomov@gmail.com> >>> --- >>> drivers/block/nbd.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >> >> Oops, thanks Ilya >> >> Reviewed-by: Josef Bacik <jbacik@fb.com> > > Since config is referenced, why isn't this done in nbd_config_put() > instead? Josef's 5ea8d10802ec added that nbd->config = NULL to nbd_reset(), so I followed his lead. It could be done nbd_config_put() -- nbd_reset() call in nbd_dev_add() looks like a no-op to me. Josef? Thanks, Ilya
On Tue, May 23, 2017 at 04:27:56PM +0200, Ilya Dryomov wrote: > On Tue, May 23, 2017 at 4:16 PM, Jens Axboe <axboe@fb.com> wrote: > > On 05/23/2017 08:14 AM, Josef Bacik wrote: > >> On Tue, May 23, 2017 at 12:38:31PM +0200, Ilya Dryomov wrote: > >>> nbd_config is allocated in nbd_alloc_config(), but never freed. > >>> > >>> Fixes: 5ea8d10802ec ("nbd: separate out the config information") > >>> Signed-off-by: Ilya Dryomov <idryomov@gmail.com> > >>> --- > >>> drivers/block/nbd.c | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >> > >> Oops, thanks Ilya > >> > >> Reviewed-by: Josef Bacik <jbacik@fb.com> > > > > Since config is referenced, why isn't this done in nbd_config_put() > > instead? > > Josef's 5ea8d10802ec added that nbd->config = NULL to nbd_reset(), so > I followed his lead. It could be done nbd_config_put() -- nbd_reset() > call in nbd_dev_add() looks like a no-op to me. Josef? > Yeah it used to be called more, but when I moved the config to be ref counted it lost it's old uses and I just didn't notice enough to delete it. Just fold that into nbd_config_put() and call it a day. Thanks, Josef
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 9a7bb2c29447..882fb9efbab1 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -940,6 +940,7 @@ static int nbd_reconnect_socket(struct nbd_device *nbd, unsigned long arg) /* Reset all properties of an NBD device */ static void nbd_reset(struct nbd_device *nbd) { + kfree(nbd->config); nbd->config = NULL; nbd->tag_set.timeout = 0; queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
nbd_config is allocated in nbd_alloc_config(), but never freed. Fixes: 5ea8d10802ec ("nbd: separate out the config information") Signed-off-by: Ilya Dryomov <idryomov@gmail.com> --- drivers/block/nbd.c | 1 + 1 file changed, 1 insertion(+)