Re: [PATCH] mm/userfaultfd: store the child's new uffd context in the wait queue entry

["Jose A. Perez de Azpillaga" <azpijr@gmail.com>]

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