[PATCH net] rds: ib: Clear the sg list when mapping an MR fails

[PATCH net] rds: ib: Clear the sg list when mapping an MR fails

[Dongliang Qin]

rds_ib_map_frmr() stores the caller's scatterlist in the MR before DMA
mapping and registration can fail. On failure, __rds_rdma_map() unpins
the pages and frees the scatterlist, but rds_ib_free_frmr() can still
return the MR to the pool with the stale pointer set.

This leaves the pool with a dangling scatterlist and can lead to local
privilege escalation. KASAN detects the resulting use-after-free when the
MR is later torn down:

BUG: KASAN: slab-use-after-free in __rds_ib_teardown_mr
Read of size 8

Call Trace:
 __rds_ib_teardown_mr
 rds_ib_unreg_frmr
 rds_ib_flush_mr_pool
 rds_ib_flush_mrs
 rds_free_mr
 rds_setsockopt

Clear the scatterlist fields before returning an error so the caller
retains sole ownership of the scatterlist and its pinned pages. Route a
zero-length DMA mapping through the same cleanup path, and skip unmap
when no mapping was created.

Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
Cc: stable@vger.kernel.org
Signed-off-by: Dongliang Qin <cccccccccccc777777@gmail.com>
---
 net/rds/ib_frmr.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
index bd8611911..fea387952 100644
--- a/net/rds/ib_frmr.c
+++ b/net/rds/ib_frmr.c
@@ -213,7 +213,8 @@ static int rds_ib_map_frmr(struct rds_ib_device *rds_ibdev,
 					 DMA_BIDIRECTIONAL);
 	if (unlikely(!ibmr->sg_dma_len)) {
 		pr_warn("RDS/IB: %s failed!\n", __func__);
-		return -EBUSY;
+		ret = -EBUSY;
+		goto out_unmap;
 	}
 
 	frmr->sg_byte_len = 0;
@@ -261,9 +262,13 @@ static int rds_ib_map_frmr(struct rds_ib_device *rds_ibdev,
 	return ret;
 
 out_unmap:
-	ib_dma_unmap_sg(rds_ibdev->dev, ibmr->sg, ibmr->sg_len,
-			DMA_BIDIRECTIONAL);
-	ibmr->sg_dma_len = 0;
+	if (ibmr->sg_dma_len) {
+		ib_dma_unmap_sg(rds_ibdev->dev, ibmr->sg, ibmr->sg_len,
+				DMA_BIDIRECTIONAL);
+		ibmr->sg_dma_len = 0;
+	}
+	ibmr->sg = NULL;
+	ibmr->sg_len = 0;
 	return ret;
 }
 
-- 
2.43.0

Re: [PATCH net] rds: ib: Clear the sg list when mapping an MR fails

[Simon Horman]

On Sun, Sep 20, 2026 at 02:34:49PM +0800, Dongliang Qin wrote:
> rds_ib_map_frmr() stores the caller's scatterlist in the MR before DMA
> mapping and registration can fail. On failure, __rds_rdma_map() unpins
> the pages and frees the scatterlist, but rds_ib_free_frmr() can still
> return the MR to the pool with the stale pointer set.
> 
> This leaves the pool with a dangling scatterlist and can lead to local
> privilege escalation. KASAN detects the resulting use-after-free when the
> MR is later torn down:
> 
> BUG: KASAN: slab-use-after-free in __rds_ib_teardown_mr
> Read of size 8
> 
> Call Trace:
>  __rds_ib_teardown_mr
>  rds_ib_unreg_frmr
>  rds_ib_flush_mr_pool
>  rds_ib_flush_mrs
>  rds_free_mr
>  rds_setsockopt
> 
> Clear the scatterlist fields before returning an error so the caller
> retains sole ownership of the scatterlist and its pinned pages. Route a
> zero-length DMA mapping through the same cleanup path, and skip unmap
> when no mapping was created.
> 
> Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dongliang Qin <cccccccccccc777777@gmail.com>
> ---
>  net/rds/ib_frmr.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> index bd8611911..fea387952 100644
> --- a/net/rds/ib_frmr.c
> +++ b/net/rds/ib_frmr.c
> @@ -213,7 +213,8 @@ static int rds_ib_map_frmr(struct rds_ib_device *rds_ibdev,
>  					 DMA_BIDIRECTIONAL);
>  	if (unlikely(!ibmr->sg_dma_len)) {
>  		pr_warn("RDS/IB: %s failed!\n", __func__);
> -		return -EBUSY;
> +		ret = -EBUSY;
> +		goto out_unmap;
>  	}

Hi Dongliang,

This approach leads to the complexity of a condition in the out_unmap
label.  I wonder if a cleaner approach would be to, instead, only store the
scatterlist in the MR once mapping has succeeded.

With such an approach I think that ibmr->sg and ibmr->sg_len would still
need to be cleared in the error path. But neither the condition in the
out_unmap label nor the hunk above would not be necessary. And I wonder
if that would be a cleaner approach.

>  
>  	frmr->sg_byte_len = 0;
> @@ -261,9 +262,13 @@ static int rds_ib_map_frmr(struct rds_ib_device *rds_ibdev,
>  	return ret;
>  
>  out_unmap:
> -	ib_dma_unmap_sg(rds_ibdev->dev, ibmr->sg, ibmr->sg_len,
> -			DMA_BIDIRECTIONAL);
> -	ibmr->sg_dma_len = 0;
> +	if (ibmr->sg_dma_len) {
> +		ib_dma_unmap_sg(rds_ibdev->dev, ibmr->sg, ibmr->sg_len,
> +				DMA_BIDIRECTIONAL);
> +		ibmr->sg_dma_len = 0;
> +	}
> +	ibmr->sg = NULL;
> +	ibmr->sg_len = 0;
>  	return ret;
>  }
>  
> -- 
> 2.43.0
> 

Re: [PATCH net] rds: ib: Clear the sg list when mapping an MR fails

[Allison Henderson]

On Mon, 2026-09-21 at 17:57 +0100, Simon Horman wrote:
> On Sun, Sep 20, 2026 at 02:34:49PM +0800, Dongliang Qin wrote:
> > rds_ib_map_frmr() stores the caller's scatterlist in the MR before DMA
> > mapping and registration can fail. On failure, __rds_rdma_map() unpins
> > the pages and frees the scatterlist, but rds_ib_free_frmr() can still
> > return the MR to the pool with the stale pointer set.
> > 
> > This leaves the pool with a dangling scatterlist and can lead to local
> > privilege escalation. KASAN detects the resulting use-after-free when the
> > MR is later torn down:
> > 
> > BUG: KASAN: slab-use-after-free in __rds_ib_teardown_mr
> > Read of size 8
> > 
> > Call Trace:
> >  __rds_ib_teardown_mr
> >  rds_ib_unreg_frmr
> >  rds_ib_flush_mr_pool
> >  rds_ib_flush_mrs
> >  rds_free_mr
> >  rds_setsockopt
> > 
> > Clear the scatterlist fields before returning an error so the caller
> > retains sole ownership of the scatterlist and its pinned pages. Route a
> > zero-length DMA mapping through the same cleanup path, and skip unmap
> > when no mapping was created.
> > 
> > Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Dongliang Qin <cccccccccccc777777@gmail.com>
> > ---
> >  net/rds/ib_frmr.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
> > index bd8611911..fea387952 100644
> > --- a/net/rds/ib_frmr.c
> > +++ b/net/rds/ib_frmr.c
> > @@ -213,7 +213,8 @@ static int rds_ib_map_frmr(struct rds_ib_device *rds_ibdev,
> >  					 DMA_BIDIRECTIONAL);
> >  	if (unlikely(!ibmr->sg_dma_len)) {
> >  		pr_warn("RDS/IB: %s failed!\n", __func__);
> > -		return -EBUSY;
> > +		ret = -EBUSY;
> > +		goto out_unmap;
> >  	}
> 
> Hi Dongliang,
> 
> This approach leads to the complexity of a condition in the out_unmap
> label.  I wonder if a cleaner approach would be to, instead, only store the
> scatterlist in the MR once mapping has succeeded.
> 
> With such an approach I think that ibmr->sg and ibmr->sg_len would still
> need to be cleared in the error path. But neither the condition in the
> out_unmap label nor the hunk above would not be necessary. And I wonder
> if that would be a cleaner approach.

That sounds reasonable, the rds_ib_teardown_mr() at the top already leaves ibmr->sg
and ibmr->sg_len cleared.  So if we move the ib_dma_map_sg() below it and do the check
and goto there, then we dont need the unmap in the out path.  Just clear ibmr->sg and
ibmr->sg_len.  I think that would be a lot simpler.

Thanks for working on this!
Allison

> 
> >  
> >  	frmr->sg_byte_len = 0;
> > @@ -261,9 +262,13 @@ static int rds_ib_map_frmr(struct rds_ib_device *rds_ibdev,
> >  	return ret;
> >  
> >  out_unmap:
> > -	ib_dma_unmap_sg(rds_ibdev->dev, ibmr->sg, ibmr->sg_len,
> > -			DMA_BIDIRECTIONAL);
> > -	ibmr->sg_dma_len = 0;
> > +	if (ibmr->sg_dma_len) {
> > +		ib_dma_unmap_sg(rds_ibdev->dev, ibmr->sg, ibmr->sg_len,
> > +				DMA_BIDIRECTIONAL);
> > +		ibmr->sg_dma_len = 0;
> > +	}
> > +	ibmr->sg = NULL;
> > +	ibmr->sg_len = 0;
> >  	return ret;
> >  }
> >  
> > -- 
> > 2.43.0
> >