mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mm/userfaultfd: store the child's new uffd context in the wait queue entry
@ 2026-10-31 15:47 Tal Zussman
  2026-10-31 20:13 ` Jose A. Perez de Azpillaga
  2026-11-01  8:26 ` Mike Rapoport
  0 siblings, 2 replies; 3+ messages in thread
From: Tal Zussman @ 2026-10-31 15:47 UTC (permalink / raw)
  To: Andrew Morton, Mike Rapoport, Peter Xu
  Cc: linux-mm, linux-kernel, Tal Zussman

A UFFD_EVENT_FORK message cannot carry the child's fd when it is queued,
because the fd has to be installed in the fd table of whichever task
reads the event. The forking task has to hand the reader the new context
itself. It does that by casting the child's userfaultfd_ctx pointer into
the arg.reserved.reserved1 field of the UAPI struct uffd_msg, which the
readers cast back.

That temporarily puts a kernel pointer in a struct that is copied to
userspace, and results in ugly casts when passing the uffd context
around. While resolve_userfault_fork() currently zeroes reserved1 before
it is exposed to userspace, this unnecessarily risks leaking a kernel
pointer to userspace if future modifications are buggy.

Store the fork_ctx pointer in struct userfaultfd_wait_queue instead,
which is kernel-internal and passed around anyway. The casts and the
zeroing go away, at the (negligible) cost of 8 more bytes on the
stack.

No functional change.

Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
 mm/userfaultfd.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 3b71712edab4..cae70df89c00 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -2430,6 +2430,8 @@ struct userfaultfd_wait_queue {
 	struct uffd_msg msg;
 	wait_queue_entry_t wq;
 	struct userfaultfd_ctx *ctx;
+	/* UFFD_EVENT_FORK: the context created for the child */
+	struct userfaultfd_ctx *fork_ctx;
 	bool waken;
 };
 
@@ -3006,14 +3008,8 @@ static void userfaultfd_event_wait_completion(struct userfaultfd_ctx *ctx,
 			 * didn't.
 			 */
 			__remove_wait_queue(&ctx->event_wqh, &ewq->wq);
-			if (ewq->msg.event == UFFD_EVENT_FORK) {
-				struct userfaultfd_ctx *new;
-
-				new = (struct userfaultfd_ctx *)
-					(unsigned long)
-					ewq->msg.arg.reserved.reserved1;
-				release_new_ctx = new;
-			}
+			if (ewq->msg.event == UFFD_EVENT_FORK)
+				release_new_ctx = ewq->fork_ctx;
 			break;
 		}
 
@@ -3111,7 +3107,7 @@ static void dup_fctx(struct userfaultfd_fork_ctx *fctx)
 	msg_init(&ewq.msg);
 
 	ewq.msg.event = UFFD_EVENT_FORK;
-	ewq.msg.arg.reserved.reserved1 = (unsigned long)fctx->new;
+	ewq.fork_ctx = fctx->new;
 
 	userfaultfd_event_wait_completion(ctx, &ewq);
 }
@@ -3404,7 +3400,6 @@ static int resolve_userfault_fork(struct userfaultfd_ctx *new,
 	if (fd < 0)
 		return fd;
 
-	msg->arg.reserved.reserved1 = 0;
 	msg->arg.fork.ufd = fd;
 	return 0;
 }
@@ -3482,9 +3477,7 @@ static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait,
 			*msg = uwq->msg;
 
 			if (uwq->msg.event == UFFD_EVENT_FORK) {
-				fork_nctx = (struct userfaultfd_ctx *)
-					(unsigned long)
-					uwq->msg.arg.reserved.reserved1;
+				fork_nctx = uwq->fork_ctx;
 				list_move(&uwq->wq.entry, &fork_event);
 				/*
 				 * fork_nctx can be freed as soon as

---
base-commit: 243e8cdab64129d6207c398f8cf5fbb1962ff7e1
change-id: 20261031-uffd-fork-ctx-23fe063a5cde

Best regards,
--  
Tal Zussman <tz2294@columbia.edu>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] mm/userfaultfd: store the child's new uffd context in the wait queue entry
  2026-10-31 15:47 [PATCH] mm/userfaultfd: store the child's new uffd context in the wait queue entry Tal Zussman
@ 2026-10-31 20:13 ` Jose A. Perez de Azpillaga
  2026-11-01  8:26 ` Mike Rapoport
  1 sibling, 0 replies; 3+ messages in thread
From: Jose A. Perez de Azpillaga @ 2026-10-31 20:13 UTC (permalink / raw)
  To: Tal Zussman, Andrew Morton, Mike Rapoport, Peter Xu
  Cc: linux-mm, linux-kernel

On 10/31/26 4:47 PM, Tal Zussman wrote:
> A UFFD_EVENT_FORK message cannot carry the child's fd when it is queued,
> because the fd has to be installed in the fd table of whichever task
> reads the event. The forking task has to hand the reader the new context
> itself. It does that by casting the child's userfaultfd_ctx pointer into
> the arg.reserved.reserved1 field of the UAPI struct uffd_msg, which the
> readers cast back.
> 
> That temporarily puts a kernel pointer in a struct that is copied to
> userspace, and results in ugly casts when passing the uffd context
> around. While resolve_userfault_fork() currently zeroes reserved1 before
> it is exposed to userspace, this unnecessarily risks leaking a kernel
> pointer to userspace if future modifications are buggy.
> 
> Store the fork_ctx pointer in struct userfaultfd_wait_queue instead,
> which is kernel-internal and passed around anyway. The casts and the
> zeroing go away, at the (negligible) cost of 8 more bytes on the
> stack.
> 
> No functional change.
> 
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> ---
>  mm/userfaultfd.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> index 3b71712edab4..cae70df89c00 100644
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -2430,6 +2430,8 @@ struct userfaultfd_wait_queue {
>  	struct uffd_msg msg;
>  	wait_queue_entry_t wq;
>  	struct userfaultfd_ctx *ctx;
> +	/* UFFD_EVENT_FORK: the context created for the child */
> +	struct userfaultfd_ctx *fork_ctx;
>  	bool waken;
>  };
>  
> @@ -3006,14 +3008,8 @@ static void userfaultfd_event_wait_completion(struct userfaultfd_ctx *ctx,
>  			 * didn't.
>  			 */
>  			__remove_wait_queue(&ctx->event_wqh, &ewq->wq);
> -			if (ewq->msg.event == UFFD_EVENT_FORK) {
> -				struct userfaultfd_ctx *new;
> -
> -				new = (struct userfaultfd_ctx *)
> -					(unsigned long)
> -					ewq->msg.arg.reserved.reserved1;
> -				release_new_ctx = new;
> -			}
> +			if (ewq->msg.event == UFFD_EVENT_FORK)
> +				release_new_ctx = ewq->fork_ctx;
>  			break;
>  		}
>  
> @@ -3111,7 +3107,7 @@ static void dup_fctx(struct userfaultfd_fork_ctx *fctx)
>  	msg_init(&ewq.msg);
>  
>  	ewq.msg.event = UFFD_EVENT_FORK;
> -	ewq.msg.arg.reserved.reserved1 = (unsigned long)fctx->new;
> +	ewq.fork_ctx = fctx->new;
>  
>  	userfaultfd_event_wait_completion(ctx, &ewq);
>  }
> @@ -3404,7 +3400,6 @@ static int resolve_userfault_fork(struct userfaultfd_ctx *new,
>  	if (fd < 0)
>  		return fd;
>  
> -	msg->arg.reserved.reserved1 = 0;
>  	msg->arg.fork.ufd = fd;
>  	return 0;
>  }
> @@ -3482,9 +3477,7 @@ static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait,
>  			*msg = uwq->msg;
>  
>  			if (uwq->msg.event == UFFD_EVENT_FORK) {
> -				fork_nctx = (struct userfaultfd_ctx *)
> -					(unsigned long)
> -					uwq->msg.arg.reserved.reserved1;
> +				fork_nctx = uwq->fork_ctx;
>  				list_move(&uwq->wq.entry, &fork_event);
>  				/*
>  				 * fork_nctx can be freed as soon as
> 
this LGTM:

Reviewed-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>

-- 
cheers,
jose a. p-a

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] mm/userfaultfd: store the child's new uffd context in the wait queue entry
  2026-10-31 15:47 [PATCH] mm/userfaultfd: store the child's new uffd context in the wait queue entry Tal Zussman
  2026-10-31 20:13 ` Jose A. Perez de Azpillaga
@ 2026-11-01  8:26 ` Mike Rapoport
  1 sibling, 0 replies; 3+ messages in thread
From: Mike Rapoport @ 2026-11-01  8:26 UTC (permalink / raw)
  To: Tal Zussman; +Cc: Andrew Morton, Peter Xu, linux-mm, linux-kernel

On Sat, Oct 31, 2026 at 11:47:48AM -0400, Tal Zussman wrote:
> A UFFD_EVENT_FORK message cannot carry the child's fd when it is queued,
> because the fd has to be installed in the fd table of whichever task
> reads the event. The forking task has to hand the reader the new context
> itself. It does that by casting the child's userfaultfd_ctx pointer into
> the arg.reserved.reserved1 field of the UAPI struct uffd_msg, which the
> readers cast back.
> 
> That temporarily puts a kernel pointer in a struct that is copied to
> userspace, and results in ugly casts when passing the uffd context
> around. While resolve_userfault_fork() currently zeroes reserved1 before
> it is exposed to userspace, this unnecessarily risks leaking a kernel
> pointer to userspace if future modifications are buggy.
> 
> Store the fork_ctx pointer in struct userfaultfd_wait_queue instead,
> which is kernel-internal and passed around anyway. The casts and the
> zeroing go away, at the (negligible) cost of 8 more bytes on the
> stack.
> 
> No functional change.
> 
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  mm/userfaultfd.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)

-- 
Sincerely yours,
Mike.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-01  8:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-31 15:47 [PATCH] mm/userfaultfd: store the child's new uffd context in the wait queue entry Tal Zussman
2026-10-31 20:13 ` Jose A. Perez de Azpillaga
2026-11-01  8:26 ` Mike Rapoport

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®