Re: [PATCH] net/rds: Do not unpin a page after a failed atomic pin

[Allison Henderson]

On Thu, 2026-10-29 at 21:21 -0700, Kyle Zeng wrote:
> rds_cmsg_atomic() assumes that a non-NULL output from rds_pin_pages()
> represents a pin that it owns. However, pin_user_pages_fast() can roll
> back a fast-GUP pin after racing fork() and then fail in the slow path,
> for example after a concurrent mprotect(PROT_NONE). The output slot can
> still contain the already-unpinned page.
> 
> The common error path calls unpin_user_page() on that stale pointer.
> This can consume another subsystem's pin and allow the page to be
> freed while it is still in use.
> 
> Clear page when rds_pin_pages() does not return one. The error path
> continues to release the page on errors after a successful pin, and
> successful requests retain their existing ownership and behavior. All
> four RDS atomic ancillary operations share this path.
> 
> Signed-off-by: Kyle Zeng <kylebot@xxxxxxxxxx>

Hi Kyle,

Thanks for the patch, the fix itself looks good to me.  Some things we need
add to move you forward:  First, your subject line specify which branch the
patch is applied to.  Since this is a fix for code already in mainline, we 
should target the net tree.  So rebase the patch on the latest net branch and
format the subject line like this:

  [PATCH net v2] net/rds: Do not unpin a page after a failed atomic pin

Also, since this patch is a bug fix, it should cc stable@xxxxxxxxxxxxxxx and
carry a fixes tag that identifies which commit the patch is fixing.  For this
fix, I think we want 15133f6e67d8:

  Fixes: 15133f6e67d8 ("RDS: Implement atomic operations")

Lastly, remember to add a short change log below the --- line.  Just something
brief like this:

  v2: Target net, add Fixes and Cc stable tags


> ---
> I have a reproducer for this issue and can share via DM

Thanks, could you post it on-list with the v2?  Either below the --- line or as
a reply.  That way others can see and verify the fix too.

Thank you!
Allison

> 
>  net/rds/rdma.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/net/rds/rdma.c b/net/rds/rdma.c
> index 078090d292fa..71b57abb96c4 100644
> --- a/net/rds/rdma.c
> +++ b/net/rds/rdma.c
> @@ -957,8 +957,11 @@ int rds_cmsg_atomic(struct rds_sock *rs, struct rds_message *rm,
>  	}
>  
>  	ret = rds_pin_pages(args->local_addr, 1, &page, 1);
> -	if (ret != 1)
> +	if (ret != 1) {
> +		/* A failed pin can leave a stale page pointer. */
> +		page = NULL;
>  		goto err;
> +	}
>  	ret = 0;
>  
>  	sg_set_page(rm->atomic.op_sg, page, 8, offset_in_page(args->local_addr));
> 
> base-commit: d89e1d186ec9271bf78e62771779e9fc03bf0fc3