[v2] RDMA/rtrs: Fix use-after-free in path files cleanup

[PATCH v2] RDMA/rtrs: Fix use-after-free in path files cleanup

Posted by Guangshuo Li 1 month ago

Once kobject_put() is called on srv_path->kobj, the release callback may be
triggered and srv_path may be freed. Therefore, srv_path must not be used
after kobject_put(&srv_path->kobj).

Both rtrs_srv_create_path_files() and rtrs_srv_destroy_path_files()
currently call rtrs_srv_destroy_once_sysfs_root_folders(srv_path) after
kobject_put(&srv_path->kobj). Although the call site only passes srv_path
as an argument, rtrs_srv_destroy_once_sysfs_root_folders() dereferences it
internally to access srv_path->srv. If kobject_put() has already freed
srv_path, this results in a use-after-free.

Move rtrs_srv_destroy_once_sysfs_root_folders() before kobject_put(), so
srv_path remains valid while the helper accesses it.

This issue was found by a static analysis tool I am developing.

Fixes: ae4c81644e91 ("RDMA/rtrs-srv: Rename rtrs_srv_sess to rtrs_srv_path")
Acked-by: Md Haris Iqbal <haris.iqbal@ionos.com>
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v2:
  - Clarify that the use-after-free happens inside
    rtrs_srv_destroy_once_sysfs_root_folders(), which dereferences srv_path
    after kobject_put() may have freed it.
  - No code changes.

 drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
index 51727c7d710c..c9ba9d2d0eb3 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
@@ -295,8 +295,8 @@ int rtrs_srv_create_path_files(struct rtrs_srv_path *srv_path)
 put_kobj:
 	kobject_del(&srv_path->kobj);
 destroy_root:
-	kobject_put(&srv_path->kobj);
 	rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
+	kobject_put(&srv_path->kobj);
 
 	return err;
 }
@@ -312,8 +312,8 @@ void rtrs_srv_destroy_path_files(struct rtrs_srv_path *srv_path)
 
 	if (srv_path->kobj.state_in_sysfs) {
 		sysfs_remove_group(&srv_path->kobj, &rtrs_srv_path_attr_group);
-		kobject_put(&srv_path->kobj);
 		rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
+		kobject_put(&srv_path->kobj);
 	}
 
 }
-- 
2.43.0

Re: [PATCH v2] RDMA/rtrs: Fix use-after-free in path files cleanup

Posted by Leon Romanovsky 4 weeks, 1 day ago

On Mon, May 11, 2026 at 09:08:04PM +0800, Guangshuo Li wrote:
> Once kobject_put() is called on srv_path->kobj, the release callback may be
> triggered and srv_path may be freed. Therefore, srv_path must not be used
> after kobject_put(&srv_path->kobj).
> 
> Both rtrs_srv_create_path_files() and rtrs_srv_destroy_path_files()
> currently call rtrs_srv_destroy_once_sysfs_root_folders(srv_path) after
> kobject_put(&srv_path->kobj). Although the call site only passes srv_path
> as an argument, rtrs_srv_destroy_once_sysfs_root_folders() dereferences it
> internally to access srv_path->srv. If kobject_put() has already freed
> srv_path, this results in a use-after-free.
> 
> Move rtrs_srv_destroy_once_sysfs_root_folders() before kobject_put(), so
> srv_path remains valid while the helper accesses it.

This still doesn't answer my question: how can you access memory referenced
by the srv_path pointer after it has been freed?

  1612         rtrs_srv_destroy_path_files(srv_path); <--- you released memory pointed by srv_path here
  1613
  1614         /* Notify upper layer if we are the last path */
  1615         rtrs_srv_path_down(srv_path);          <--- you are accessing memory which was already released.
  1616

Thanks

> 
> This issue was found by a static analysis tool I am developing.
> 
> Fixes: ae4c81644e91 ("RDMA/rtrs-srv: Rename rtrs_srv_sess to rtrs_srv_path")
> Acked-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> v2:
>   - Clarify that the use-after-free happens inside
>     rtrs_srv_destroy_once_sysfs_root_folders(), which dereferences srv_path
>     after kobject_put() may have freed it.
>   - No code changes.
> 
>  drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
> index 51727c7d710c..c9ba9d2d0eb3 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
> @@ -295,8 +295,8 @@ int rtrs_srv_create_path_files(struct rtrs_srv_path *srv_path)
>  put_kobj:
>  	kobject_del(&srv_path->kobj);
>  destroy_root:
> -	kobject_put(&srv_path->kobj);
>  	rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
> +	kobject_put(&srv_path->kobj);
>  
>  	return err;
>  }
> @@ -312,8 +312,8 @@ void rtrs_srv_destroy_path_files(struct rtrs_srv_path *srv_path)
>  
>  	if (srv_path->kobj.state_in_sysfs) {
>  		sysfs_remove_group(&srv_path->kobj, &rtrs_srv_path_attr_group);
> -		kobject_put(&srv_path->kobj);
>  		rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
> +		kobject_put(&srv_path->kobj);
>  	}
>  
>  }
> -- 
> 2.43.0
>

Re: [PATCH v2] RDMA/rtrs: Fix use-after-free in path files cleanup

Posted by Guangshuo Li 4 weeks, 1 day ago

Hi Leon,

Thanks, I see your point.

On Thu, 14 May 2026 at 15:18, Leon Romanovsky <leon@kernel.org> wrote:
>
> On Mon, May 11, 2026 at 09:08:04PM +0800, Guangshuo Li wrote:
> > Once kobject_put() is called on srv_path->kobj, the release callback may be
> > triggered and srv_path may be freed. Therefore, srv_path must not be used
> > after kobject_put(&srv_path->kobj).
> >
> > Both rtrs_srv_create_path_files() and rtrs_srv_destroy_path_files()
> > currently call rtrs_srv_destroy_once_sysfs_root_folders(srv_path) after
> > kobject_put(&srv_path->kobj). Although the call site only passes srv_path
> > as an argument, rtrs_srv_destroy_once_sysfs_root_folders() dereferences it
> > internally to access srv_path->srv. If kobject_put() has already freed
> > srv_path, this results in a use-after-free.
> >
> > Move rtrs_srv_destroy_once_sysfs_root_folders() before kobject_put(), so
> > srv_path remains valid while the helper accesses it.
>
> This still doesn't answer my question: how can you access memory referenced
> by the srv_path pointer after it has been freed?
>
>   1612         rtrs_srv_destroy_path_files(srv_path); <--- you released memory pointed by srv_path here
>   1613
>   1614         /* Notify upper layer if we are the last path */
>   1615         rtrs_srv_path_down(srv_path);          <--- you are accessing memory which was already released.
>   1616
>
> Thanks
>
> >
> > This issue was found by a static analysis tool I am developing.
> >
> > Fixes: ae4c81644e91 ("RDMA/rtrs-srv: Rename rtrs_srv_sess to rtrs_srv_path")
> > Acked-by: Md Haris Iqbal <haris.iqbal@ionos.com>
> > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> > ---
> > v2:
> >   - Clarify that the use-after-free happens inside
> >     rtrs_srv_destroy_once_sysfs_root_folders(), which dereferences srv_path
> >     after kobject_put() may have freed it.
> >   - No code changes.
> >
> >  drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
> > index 51727c7d710c..c9ba9d2d0eb3 100644
> > --- a/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
> > +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv-sysfs.c
> > @@ -295,8 +295,8 @@ int rtrs_srv_create_path_files(struct rtrs_srv_path *srv_path)
> >  put_kobj:
> >       kobject_del(&srv_path->kobj);
> >  destroy_root:
> > -     kobject_put(&srv_path->kobj);
> >       rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
> > +     kobject_put(&srv_path->kobj);
> >
> >       return err;
> >  }
> > @@ -312,8 +312,8 @@ void rtrs_srv_destroy_path_files(struct rtrs_srv_path *srv_path)
> >
> >       if (srv_path->kobj.state_in_sysfs) {
> >               sysfs_remove_group(&srv_path->kobj, &rtrs_srv_path_attr_group);
> > -             kobject_put(&srv_path->kobj);
> >               rtrs_srv_destroy_once_sysfs_root_folders(srv_path);
> > +             kobject_put(&srv_path->kobj);
> >       }
> >
> >  }
> > --
> > 2.43.0
> >

You are right that in the normal rtrs_srv_destroy_path_files() path,
the kobject_put() there cannot be treated as unconditionally freeing
srv_path; otherwise the following accesses such as rtrs_srv_path_down()
would already be a problem.

The case I intended to fix is the error path in
rtrs_srv_create_path_files().

For example, if rtrs_srv_create_once_sysfs_root_folders() succeeds and
kobject_init_and_add() succeeds, but a later step such as
sysfs_create_group() or rtrs_srv_create_stats_files() fails, the error
path does:

kobject_del(&srv_path->kobj);
kobject_put(&srv_path->kobj);
rtrs_srv_destroy_once_sysfs_root_folders(srv_path);

In this failure path, rtrs_srv_create_path_files() has not returned
success, so the later successful-path reference handling is not reached.
Therefore kobject_put(&srv_path->kobj) may invoke rtrs_srv_release()
and free srv_path. The following call to
rtrs_srv_destroy_once_sysfs_root_folders(srv_path) then dereferences
srv_path internally to access srv_path->srv.

So the UAF I am addressing is in the create failure cleanup path. My v2
commit message was too broad and made it sound as if the normal destroy
path had the same lifetime issue.

I will send a v3 clarifying that, and I can also limit the code change to
the rtrs_srv_create_path_files() error path if you prefer.

Thanks,
Guangshuo