| Message ID | 20220120103714.32108-1-linmq006@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RDMA/rtrs: Fix double free in alloc_clt | expand |
On Thu, Jan 20, 2022 at 11:37 AM Miaoqian Lin <linmq006@gmail.com> wrote: > > Callback function rtrs_clt_dev_release() in put_device() > calls kfree(clt); to free memory. We shouldn't call kfree(clt) again. > right, there is a bug, but the fix is not right. > Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality") > Signed-off-by: Miaoqian Lin <linmq006@gmail.com> > --- > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c > index 7c3f98e57889..61723f48fbd4 100644 > --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c > @@ -2741,7 +2741,7 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num, > err = device_register(&clt->dev); > if (err) { > put_device(&clt->dev); > - goto err; > + goto err_free_cpu; > } > > clt->kobj_paths = kobject_create_and_add("paths", &clt->dev.kobj); > @@ -2764,6 +2764,9 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num, > err: > free_percpu(clt->pcpu_path); > kfree(clt); > + clt->pcpu_path = NULL; > +err_free_cpu: > + free_percpu(clt->pcpu_path); this still lead to use after free, because clt could be free already. the right fix should move the free_percpu() to the release call back before kfree(clt); > return ERR_PTR(err); > } > > -- > 2.17.1 >
On Thu, Jan 20, 2022 at 10:37:14AM +0000, Miaoqian Lin wrote: > Callback function rtrs_clt_dev_release() in put_device() > calls kfree(clt); to free memory. We shouldn't call kfree(clt) again. > > Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality") > Signed-off-by: Miaoqian Lin <linmq006@gmail.com> > --- > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c > index 7c3f98e57889..61723f48fbd4 100644 > --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c > +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c > @@ -2741,7 +2741,7 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num, > err = device_register(&clt->dev); > if (err) { > put_device(&clt->dev); > - goto err; > + goto err_free_cpu; > } > > clt->kobj_paths = kobject_create_and_add("paths", &clt->dev.kobj); > @@ -2764,6 +2764,9 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num, > err: > free_percpu(clt->pcpu_path); > kfree(clt); > + clt->pcpu_path = NULL; Like Jinpu said, it will crash, you are trying to access clt after kfree. Thanks > +err_free_cpu: > + free_percpu(clt->pcpu_path); > return ERR_PTR(err); > } > > -- > 2.17.1 >
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c index 7c3f98e57889..61723f48fbd4 100644 --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c @@ -2741,7 +2741,7 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num, err = device_register(&clt->dev); if (err) { put_device(&clt->dev); - goto err; + goto err_free_cpu; } clt->kobj_paths = kobject_create_and_add("paths", &clt->dev.kobj); @@ -2764,6 +2764,9 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num, err: free_percpu(clt->pcpu_path); kfree(clt); + clt->pcpu_path = NULL; +err_free_cpu: + free_percpu(clt->pcpu_path); return ERR_PTR(err); }
Callback function rtrs_clt_dev_release() in put_device() calls kfree(clt); to free memory. We shouldn't call kfree(clt) again. Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality") Signed-off-by: Miaoqian Lin <linmq006@gmail.com> --- drivers/infiniband/ulp/rtrs/rtrs-clt.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)