* [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case
@ 2022-02-09 15:14 Md Haris Iqbal
2022-02-09 15:14 ` [PATCH 2/2] RDMA/rtrs-clt: Move free_permit from free_clt to rtrs_clt_close Md Haris Iqbal
2022-02-16 18:04 ` [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case Jason Gunthorpe
0 siblings, 2 replies; 9+ messages in thread
From: Md Haris Iqbal @ 2022-02-09 15:14 UTC (permalink / raw)
To: linux-rdma; +Cc: bvanassche, leon, jgg, haris.iqbal, jinpu.wang
Callback function rtrs_clt_dev_release() for put_device() calls kfree(clt)
to free memory. We shouldn't call kfree(clt) again, and we can't use the
clt after kfree too.
Replace device_register with device_initialize and device_add so that
dev_set_name can be used appropriately.
Move mutex_destroy to release function so it can be called in alloc_clt err
path.
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 37 ++++++++++++++------------
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index b696aa4abae4..d20bad345eff 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2685,6 +2685,8 @@ static void rtrs_clt_dev_release(struct device *dev)
struct rtrs_clt_sess *clt = container_of(dev, struct rtrs_clt_sess,
dev);
+ mutex_destroy(&clt->paths_ev_mutex);
+ mutex_destroy(&clt->paths_mutex);
kfree(clt);
}
@@ -2714,6 +2716,8 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
return ERR_PTR(-ENOMEM);
}
+ clt->dev.class = rtrs_clt_dev_class;
+ clt->dev.release = rtrs_clt_dev_release;
uuid_gen(&clt->paths_uuid);
INIT_LIST_HEAD_RCU(&clt->paths_list);
clt->paths_num = paths_num;
@@ -2730,43 +2734,41 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
init_waitqueue_head(&clt->permits_wait);
mutex_init(&clt->paths_ev_mutex);
mutex_init(&clt->paths_mutex);
+ device_initialize(&clt->dev);
- clt->dev.class = rtrs_clt_dev_class;
- clt->dev.release = rtrs_clt_dev_release;
err = dev_set_name(&clt->dev, "%s", sessname);
if (err)
- goto err;
+ goto err_put;
+
/*
* Suppress user space notification until
* sysfs files are created
*/
dev_set_uevent_suppress(&clt->dev, true);
- err = device_register(&clt->dev);
- if (err) {
- put_device(&clt->dev);
- goto err;
- }
+ err = device_add(&clt->dev);
+ if (err)
+ goto err_put;
clt->kobj_paths = kobject_create_and_add("paths", &clt->dev.kobj);
if (!clt->kobj_paths) {
err = -ENOMEM;
- goto err_dev;
+ goto err_del;
}
err = rtrs_clt_create_sysfs_root_files(clt);
if (err) {
kobject_del(clt->kobj_paths);
kobject_put(clt->kobj_paths);
- goto err_dev;
+ goto err_del;
}
dev_set_uevent_suppress(&clt->dev, false);
kobject_uevent(&clt->dev.kobj, KOBJ_ADD);
return clt;
-err_dev:
- device_unregister(&clt->dev);
-err:
+err_del:
+ device_del(&clt->dev);
+err_put:
free_percpu(clt->pcpu_path);
- kfree(clt);
+ put_device(&clt->dev);
return ERR_PTR(err);
}
@@ -2774,9 +2776,10 @@ static void free_clt(struct rtrs_clt_sess *clt)
{
free_permits(clt);
free_percpu(clt->pcpu_path);
- mutex_destroy(&clt->paths_ev_mutex);
- mutex_destroy(&clt->paths_mutex);
- /* release callback will free clt in last put */
+
+ /*
+ * release callback will free clt and destroy mutexes in last put
+ */
device_unregister(&clt->dev);
}
base-commit: 2f1b2820b546c1eef07d15ed73db4177c0cf6d46
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] RDMA/rtrs-clt: Move free_permit from free_clt to rtrs_clt_close
2022-02-09 15:14 [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case Md Haris Iqbal
@ 2022-02-09 15:14 ` Md Haris Iqbal
2022-02-09 18:06 ` Jinpu Wang
2022-02-16 18:04 ` [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case Jason Gunthorpe
1 sibling, 1 reply; 9+ messages in thread
From: Md Haris Iqbal @ 2022-02-09 15:14 UTC (permalink / raw)
To: linux-rdma; +Cc: bvanassche, leon, jgg, haris.iqbal, jinpu.wang
Error path of rtrs_clt_open calls free_clt, where free_permit is called.
This is wrong since error path of rtrs_clt_open does not need to call
free_permit.
Also, moving free_permits call to rtrs_clt_close, makes it more aligned
with the call to alloc_permit in rtrs_clt_open.
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index d20bad345eff..c2c860d0c56e 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2774,7 +2774,6 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
static void free_clt(struct rtrs_clt_sess *clt)
{
- free_permits(clt);
free_percpu(clt->pcpu_path);
/*
@@ -2896,6 +2895,7 @@ void rtrs_clt_close(struct rtrs_clt_sess *clt)
rtrs_clt_destroy_path_files(clt_path, NULL);
kobject_put(&clt_path->kobj);
}
+ free_permits(clt);
free_clt(clt);
}
EXPORT_SYMBOL(rtrs_clt_close);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] RDMA/rtrs-clt: Move free_permit from free_clt to rtrs_clt_close
2022-02-09 15:14 ` [PATCH 2/2] RDMA/rtrs-clt: Move free_permit from free_clt to rtrs_clt_close Md Haris Iqbal
@ 2022-02-09 18:06 ` Jinpu Wang
0 siblings, 0 replies; 9+ messages in thread
From: Jinpu Wang @ 2022-02-09 18:06 UTC (permalink / raw)
To: Md Haris Iqbal; +Cc: linux-rdma, bvanassche, leon, jgg
On Wed, Feb 9, 2022 at 4:14 PM Md Haris Iqbal <haris.iqbal@ionos.com> wrote:
>
> Error path of rtrs_clt_open calls free_clt, where free_permit is called.
> This is wrong since error path of rtrs_clt_open does not need to call
> free_permit.
>
> Also, moving free_permits call to rtrs_clt_close, makes it more aligned
> with the call to alloc_permit in rtrs_clt_open.
>
> Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Thx!
> ---
> drivers/infiniband/ulp/rtrs/rtrs-clt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index d20bad345eff..c2c860d0c56e 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -2774,7 +2774,6 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
>
> static void free_clt(struct rtrs_clt_sess *clt)
> {
> - free_permits(clt);
> free_percpu(clt->pcpu_path);
>
> /*
> @@ -2896,6 +2895,7 @@ void rtrs_clt_close(struct rtrs_clt_sess *clt)
> rtrs_clt_destroy_path_files(clt_path, NULL);
> kobject_put(&clt_path->kobj);
> }
> + free_permits(clt);
> free_clt(clt);
> }
> EXPORT_SYMBOL(rtrs_clt_close);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case
2022-02-09 15:14 [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case Md Haris Iqbal
2022-02-09 15:14 ` [PATCH 2/2] RDMA/rtrs-clt: Move free_permit from free_clt to rtrs_clt_close Md Haris Iqbal
@ 2022-02-16 18:04 ` Jason Gunthorpe
1 sibling, 0 replies; 9+ messages in thread
From: Jason Gunthorpe @ 2022-02-16 18:04 UTC (permalink / raw)
To: Md Haris Iqbal; +Cc: linux-rdma, bvanassche, leon, jinpu.wang
On Wed, Feb 09, 2022 at 04:14:24PM +0100, Md Haris Iqbal wrote:
> Callback function rtrs_clt_dev_release() for put_device() calls kfree(clt)
> to free memory. We shouldn't call kfree(clt) again, and we can't use the
> clt after kfree too.
>
> Replace device_register with device_initialize and device_add so that
> dev_set_name can be used appropriately.
>
> Move mutex_destroy to release function so it can be called in alloc_clt err
> path.
>
> Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-clt.c | 37 ++++++++++++++------------
> 1 file changed, 20 insertions(+), 17 deletions(-)
Fixes lines for both patches?
Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case
2022-02-09 12:00 ` Haris Iqbal
@ 2022-02-09 12:25 ` Jason Gunthorpe
0 siblings, 0 replies; 9+ messages in thread
From: Jason Gunthorpe @ 2022-02-09 12:25 UTC (permalink / raw)
To: Haris Iqbal; +Cc: linux-rdma, bvanassche, leon, jinpu.wang
On Wed, Feb 09, 2022 at 01:00:32PM +0100, Haris Iqbal wrote:
> On Tue, Feb 8, 2022 at 5:48 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > On Wed, Feb 02, 2022 at 04:08:54PM +0100, Md Haris Iqbal wrote:
> > > Callback function rtrs_clt_dev_release() for put_device() calls kfree(clt)
> > > to free memory. We shouldn't call kfree(clt) again, and we can't use the
> > > clt after kfree too.
> > >
> > > Replace device_register with device_initialize and device_add so that
> > > dev_set_name can be used appropriately.
> > >
> > > Move mutex_destroy to release function so it can be called in alloc_clt err
> > > path.
> > >
> > > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > > Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
> > > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 37 ++++++++++++++------------
> > > 1 file changed, 20 insertions(+), 17 deletions(-)
> >
> > These patches don't apply, please resend them
>
> Hi Jason,
>
> I tried these 2 patches over wip/jgg-for-next (commit
> 2f1b2820b546c1eef07d15ed73db4177c0cf6d46) and it applies. Can you
> check once more if there is some other issue? Thanks.
It applied for you because you have the right 3 way information, I
don't. You need to generate and send patches against clean trees
Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case
2022-02-08 16:48 ` Jason Gunthorpe
2022-02-09 11:09 ` Haris Iqbal
@ 2022-02-09 12:00 ` Haris Iqbal
2022-02-09 12:25 ` Jason Gunthorpe
1 sibling, 1 reply; 9+ messages in thread
From: Haris Iqbal @ 2022-02-09 12:00 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: linux-rdma, bvanassche, leon, jinpu.wang
On Tue, Feb 8, 2022 at 5:48 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Wed, Feb 02, 2022 at 04:08:54PM +0100, Md Haris Iqbal wrote:
> > Callback function rtrs_clt_dev_release() for put_device() calls kfree(clt)
> > to free memory. We shouldn't call kfree(clt) again, and we can't use the
> > clt after kfree too.
> >
> > Replace device_register with device_initialize and device_add so that
> > dev_set_name can be used appropriately.
> >
> > Move mutex_destroy to release function so it can be called in alloc_clt err
> > path.
> >
> > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
> > ---
> > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 37 ++++++++++++++------------
> > 1 file changed, 20 insertions(+), 17 deletions(-)
>
> These patches don't apply, please resend them
Hi Jason,
I tried these 2 patches over wip/jgg-for-next (commit
2f1b2820b546c1eef07d15ed73db4177c0cf6d46) and it applies. Can you
check once more if there is some other issue? Thanks.
>
> Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case
2022-02-08 16:48 ` Jason Gunthorpe
@ 2022-02-09 11:09 ` Haris Iqbal
2022-02-09 12:00 ` Haris Iqbal
1 sibling, 0 replies; 9+ messages in thread
From: Haris Iqbal @ 2022-02-09 11:09 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: linux-rdma, bvanassche, leon, jinpu.wang
On Tue, Feb 8, 2022 at 5:48 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Wed, Feb 02, 2022 at 04:08:54PM +0100, Md Haris Iqbal wrote:
> > Callback function rtrs_clt_dev_release() for put_device() calls kfree(clt)
> > to free memory. We shouldn't call kfree(clt) again, and we can't use the
> > clt after kfree too.
> >
> > Replace device_register with device_initialize and device_add so that
> > dev_set_name can be used appropriately.
> >
> > Move mutex_destroy to release function so it can be called in alloc_clt err
> > path.
> >
> > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
> > ---
> > drivers/infiniband/ulp/rtrs/rtrs-clt.c | 37 ++++++++++++++------------
> > 1 file changed, 20 insertions(+), 17 deletions(-)
>
> These patches don't apply, please resend them
Sure. Will resend.
>
> Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case
2022-02-02 15:08 Md Haris Iqbal
@ 2022-02-08 16:48 ` Jason Gunthorpe
2022-02-09 11:09 ` Haris Iqbal
2022-02-09 12:00 ` Haris Iqbal
0 siblings, 2 replies; 9+ messages in thread
From: Jason Gunthorpe @ 2022-02-08 16:48 UTC (permalink / raw)
To: Md Haris Iqbal; +Cc: linux-rdma, bvanassche, leon, jinpu.wang
On Wed, Feb 02, 2022 at 04:08:54PM +0100, Md Haris Iqbal wrote:
> Callback function rtrs_clt_dev_release() for put_device() calls kfree(clt)
> to free memory. We shouldn't call kfree(clt) again, and we can't use the
> clt after kfree too.
>
> Replace device_register with device_initialize and device_add so that
> dev_set_name can be used appropriately.
>
> Move mutex_destroy to release function so it can be called in alloc_clt err
> path.
>
> Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-clt.c | 37 ++++++++++++++------------
> 1 file changed, 20 insertions(+), 17 deletions(-)
These patches don't apply, please resend them
Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case
@ 2022-02-02 15:08 Md Haris Iqbal
2022-02-08 16:48 ` Jason Gunthorpe
0 siblings, 1 reply; 9+ messages in thread
From: Md Haris Iqbal @ 2022-02-02 15:08 UTC (permalink / raw)
To: linux-rdma; +Cc: bvanassche, leon, jgg, haris.iqbal, jinpu.wang
Callback function rtrs_clt_dev_release() for put_device() calls kfree(clt)
to free memory. We shouldn't call kfree(clt) again, and we can't use the
clt after kfree too.
Replace device_register with device_initialize and device_add so that
dev_set_name can be used appropriately.
Move mutex_destroy to release function so it can be called in alloc_clt err
path.
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
---
drivers/infiniband/ulp/rtrs/rtrs-clt.c | 37 ++++++++++++++------------
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
index b696aa4abae4..d20bad345eff 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2685,6 +2685,8 @@ static void rtrs_clt_dev_release(struct device *dev)
struct rtrs_clt_sess *clt = container_of(dev, struct rtrs_clt_sess,
dev);
+ mutex_destroy(&clt->paths_ev_mutex);
+ mutex_destroy(&clt->paths_mutex);
kfree(clt);
}
@@ -2714,6 +2716,8 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
return ERR_PTR(-ENOMEM);
}
+ clt->dev.class = rtrs_clt_dev_class;
+ clt->dev.release = rtrs_clt_dev_release;
uuid_gen(&clt->paths_uuid);
INIT_LIST_HEAD_RCU(&clt->paths_list);
clt->paths_num = paths_num;
@@ -2730,43 +2734,41 @@ static struct rtrs_clt_sess *alloc_clt(const char *sessname, size_t paths_num,
init_waitqueue_head(&clt->permits_wait);
mutex_init(&clt->paths_ev_mutex);
mutex_init(&clt->paths_mutex);
+ device_initialize(&clt->dev);
- clt->dev.class = rtrs_clt_dev_class;
- clt->dev.release = rtrs_clt_dev_release;
err = dev_set_name(&clt->dev, "%s", sessname);
if (err)
- goto err;
+ goto err_put;
+
/*
* Suppress user space notification until
* sysfs files are created
*/
dev_set_uevent_suppress(&clt->dev, true);
- err = device_register(&clt->dev);
- if (err) {
- put_device(&clt->dev);
- goto err;
- }
+ err = device_add(&clt->dev);
+ if (err)
+ goto err_put;
clt->kobj_paths = kobject_create_and_add("paths", &clt->dev.kobj);
if (!clt->kobj_paths) {
err = -ENOMEM;
- goto err_dev;
+ goto err_del;
}
err = rtrs_clt_create_sysfs_root_files(clt);
if (err) {
kobject_del(clt->kobj_paths);
kobject_put(clt->kobj_paths);
- goto err_dev;
+ goto err_del;
}
dev_set_uevent_suppress(&clt->dev, false);
kobject_uevent(&clt->dev.kobj, KOBJ_ADD);
return clt;
-err_dev:
- device_unregister(&clt->dev);
-err:
+err_del:
+ device_del(&clt->dev);
+err_put:
free_percpu(clt->pcpu_path);
- kfree(clt);
+ put_device(&clt->dev);
return ERR_PTR(err);
}
@@ -2774,9 +2776,10 @@ static void free_clt(struct rtrs_clt_sess *clt)
{
free_permits(clt);
free_percpu(clt->pcpu_path);
- mutex_destroy(&clt->paths_ev_mutex);
- mutex_destroy(&clt->paths_mutex);
- /* release callback will free clt in last put */
+
+ /*
+ * release callback will free clt and destroy mutexes in last put
+ */
device_unregister(&clt->dev);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-16 18:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09 15:14 [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case Md Haris Iqbal
2022-02-09 15:14 ` [PATCH 2/2] RDMA/rtrs-clt: Move free_permit from free_clt to rtrs_clt_close Md Haris Iqbal
2022-02-09 18:06 ` Jinpu Wang
2022-02-16 18:04 ` [PATCH v3 1/2] RDMA/rtrs-clt: Fix possible double free in error case Jason Gunthorpe
-- strict thread matches above, loose matches on Subject: below --
2022-02-02 15:08 Md Haris Iqbal
2022-02-08 16:48 ` Jason Gunthorpe
2022-02-09 11:09 ` Haris Iqbal
2022-02-09 12:00 ` Haris Iqbal
2022-02-09 12:25 ` Jason Gunthorpe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.