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

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

[Kyle Zeng]

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@openai.com>
---
I have a reproducer for this issue and can share via DM

 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
-- 
2.56.0

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

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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@openai.com>

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@vger.kernel.org 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