nvme-loop: kfree(ctrl) on _create() error exit
Subsystems (2)
PATCH 1/1 nvme-loop: kfree(ctrl) on _create() error exit
ctrl is kzalloc'ed but looks like it's never kfree'ed on the goto exit cases on nvme_loop_create_ctrl().
--- drivers/nvme/target/loop.c | 1 + 1 file changed, 1 insertion(+)
drivers/nvme/target/loop.c +1 -0
@@ -673,6 +673,7 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,nvme_uninit_ctrl (&ctrl->ctrl); out_put_ctrl:nvme_put_ctrl (&ctrl->ctrl); +kfree (ctrl);if (ret >0 ) ret = -EIO ;return ERR_PTR (ret);
Signature · 1 line
2.7.4
PATCH 1/1 nvme-loop: kfree(ctrl) on _create() error exit
queued to the next round of rc fixes, thanks Jay..
PATCH 1/1 nvme-loop: kfree(ctrl) on _create() error exit
In reply to Jay Freyenseedrivers/nvme/target/loop.c static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev, · line 679
Quoted 12 lines · Jay Freyensee
drivers/nvme/target/loop.c +1
-0@@ -673,6 +673,7 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,nvme_uninit_ctrl (&ctrl->ctrl); out_put_ctrl:nvme_put_ctrl (&ctrl->ctrl); +kfree (ctrl);if (ret >0 ) ret = -EIO ;return ERR_PTR (ret);
This will give us a double free. The final put in nvme_put_ctrl calls ->free_ctrl which will free the controller.
PATCH 1/1 nvme-loop: kfree(ctrl) on _create() error exit
Quoted 12 lines · Christoph Hellwig
Quoted 11 lines · Jay Freyensee
drivers/nvme/target/loop.c +0
-0@@ -673,6 +673,7 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev, ? nvme_uninit_ctrl(&ctrl->ctrl);?out_put_ctrl:
?nvme_put_ctrl(&ctrl->ctrl);
Quoted 8 lines
Quoted 4 lines
drivers/nvme/target/loop.c +1
-0+kfree (ctrl);? if (ret > 0)
? ret =-EIO;
? returnERR_PTR(ret);This will give us a double free.??The final put in
nvme_put_ctrlcalls
->free_ctrl which will free the controller.
But doesn't nvme_put_ctrl() just take care of the nvme_ctrl? ?The kfree(ctrl) is for nvme_loop_ctrl instance being kzalloc'ed at the beginning of the function. ?I don't think this is any different than what is done for the create_ctrl() implementation in host/rdma.c?:
host/rdma.c
===========out_uninit_ctrl:
nvme_uninit_ctrl(&ctrl->ctrl); nvme_put_ctrl(&ctrl->ctrl); if (ret > 0) ret = -EIO; return ERR_PTR(ret); out_free_ctrl: kfree(ctrl); ? <-- this is for nvme_rdma_ctrl alloc return ERR_PTR(ret);
target/loop.c
========out_uninit_ctrl:
nvme_uninit_ctrl(&ctrl->ctrl); out_put_ctrl: nvme_put_ctrl(&ctrl->ctrl); kfree(ctrl); ? <-- the proposed fix for nvme_loop_ctrl alloc if (ret > 0) ret = -EIO; return ERR_PTR(ret); }
Quoted 5 lines
_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
PATCH 1/1 nvme-loop: kfree(ctrl) on _create() error exit
Quoted 3 lines · J Freyensee
But doesn't
nvme_put_ctrl()just take care of thenvme_ctrl? ?The kfree(ctrl) is fornvme_loop_ctrlinstance being kzalloc'ed at the beginning of the function.
The nvmet_ctrl structure is embedded into the nvme_loop_ctrl structure, it's not a separate allocation.
Quoted 11 lines
?I don't think this is any different than
what is done for thecreate_ctrl()implementation inhost/rdma.c?:
host/rdma.c
===========out_uninit_ctrl:nvme_uninit_ctrl(&ctrl->ctrl); nvme_put_ctrl(&ctrl->ctrl); if (ret > 0) ret = -EIO; return ERR_PTR(ret);
This is the case after nvme_init_ctrl and we just do the put and then return.
Quoted 3 lines
out_free_ctrl:kfree(ctrl); ? <-- this is for nvme_rdma_ctrl alloc return ERR_PTR(ret);
And this is the case before we did nvme_init_ctrl and we just kfree the ctrl.
Sagi Grimberg
maintainerPATCH 1/1 nvme-loop: kfree(ctrl) on _create() error exit
Quoted 3 lines
The
nvmet_ctrlstructure is embedded into thenvme_loop_ctrlstructure, it's not a separate allocation.
oops, was too easy on this one...
reverting from nvmf-4.9-rc...