[PATCH for-next] RDMA/rtrs-clt: destroy sysfs after removing session from active list

[PATCH for-next] RDMA/rtrs-clt: destroy sysfs after removing session from active list

[Gioh Kim]

A session can be removed dynamically by sysfs interface "remove_path"
that eventually calls rtrs_clt_remove_path_from_sysfs function.
The current rtrs_clt_remove_path_from_sysfs first removes the sysfs
interfaces and frees sess->stats object. Second it removes the session
from the active list.

Therefore some functions could access non-connected session and
access the freed sess->stats object even-if they check the session
status before accessing the session.
For instance rtrs_clt_request and get_next_path_min_inflight
check the session status and try to send IO to the session.
The session status could be changed when they are trying to send IO
but they could not catch the change and update the statistics information
in sess->stats object, and generate use-after-free problem.
(see: "RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading
its stats")

This patch changes the rtrs_clt_remove_path_from_sysfs to remove
the session from the active session list and then destroy the sysfs
interfaces.

Each function still should check the session status because closing
or error recovery paths can change the status.

Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality")
Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
Reviewed-by: Jack Wang <jinpu.wang@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 96029d4ec26f..cbcabc893c46 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
@@ -2861,8 +2861,8 @@ int rtrs_clt_remove_path_from_sysfs(struct rtrs_clt_sess *sess,
 	} while (!changed && old_state != RTRS_CLT_DEAD);
 
 	if (likely(changed)) {
-		rtrs_clt_destroy_sess_files(sess, sysfs_self);
 		rtrs_clt_remove_path_from_arr(sess);
+		rtrs_clt_destroy_sess_files(sess, sysfs_self);
 		kobject_put(&sess->kobj);
 	}
 
-- 
2.25.1

Re: [PATCH for-next] RDMA/rtrs-clt: destroy sysfs after removing session from active list

[Jason Gunthorpe]

On Mon, Apr 12, 2021 at 10:40:02AM +0200, Gioh Kim wrote:
> A session can be removed dynamically by sysfs interface "remove_path"
> that eventually calls rtrs_clt_remove_path_from_sysfs function.
> The current rtrs_clt_remove_path_from_sysfs first removes the sysfs
> interfaces and frees sess->stats object. Second it removes the session
> from the active list.
> 
> Therefore some functions could access non-connected session and
> access the freed sess->stats object even-if they check the session
> status before accessing the session.
> For instance rtrs_clt_request and get_next_path_min_inflight
> check the session status and try to send IO to the session.
> The session status could be changed when they are trying to send IO
> but they could not catch the change and update the statistics information
> in sess->stats object, and generate use-after-free problem.
> (see: "RDMA/rtrs-clt: Check state of the rtrs_clt_sess before reading
> its stats")
> 
> This patch changes the rtrs_clt_remove_path_from_sysfs to remove
> the session from the active session list and then destroy the sysfs
> interfaces.
> 
> Each function still should check the session status because closing
> or error recovery paths can change the status.
> 
> Fixes: 6a98d71daea1 ("RDMA/rtrs: client: main functionality")
> Signed-off-by: Gioh Kim <gi-oh.kim@ionos.com>
> Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
> ---
>  drivers/infiniband/ulp/rtrs/rtrs-clt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to for-next, thanks

Jason