Re: [PATCH 2/2] io_uring/timeout: immediate timeout arg

[Jens Axboe <axboe@kernel.dk>]

On 2/24/26 9:12 AM, Pavel Begunkov wrote:
> One the things the user has always keep in mind is that any user
> pointers they put into an SQE is not going to be read by the kernel
> until submission happens, and the user has to ensure the pointee
> stays alive until then. For example, this snippet:
> 
> void prep_timeout(struct io_uring_sqe *sqe) {
> 	struct __kernel_timespec ts = {...};
> 	prep_timeout(sqe, &ts);
> }
> 
> void submit() {
> 	sqe = get_sqe();
> 	prep_timeout(sqe);
> 	io_uring_submit();
> }
> 
> Would lead to UAF for the on stack variable ts. Instead of passing
> the timeout value as a pointer allow to store it immediately in the SQE.
> The user has to set a new flag called IORING_TIMEOUT_IMMEDIATE_ARG,
> in which case sqe->addr will be interpreted as the timeout value in ns.
> It only works with relative timeouts and rejected if set together with
> IORING_TIMEOUT_ABS out of concerns of not having enough range in u64 to
> represent a good long term API.
> 
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  include/uapi/linux/io_uring.h |  5 +++++
>  io_uring/timeout.c            | 11 +++++++++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index 6750c383a2ab..8f4de786e6e9 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -340,6 +340,10 @@ enum io_uring_op {
>  
>  /*
>   * sqe->timeout_flags
> + *
> + * IORING_TIMEOUT_IMMEDIATE_ARG:	If set, sqe->addr stores the timeout
> + *					value in nanoseconds instead of
> + *					pointing to a timespec.
>   */
>  #define IORING_TIMEOUT_ABS		(1U << 0)
>  #define IORING_TIMEOUT_UPDATE		(1U << 1)
> @@ -348,6 +352,7 @@ enum io_uring_op {
>  #define IORING_LINK_TIMEOUT_UPDATE	(1U << 4)
>  #define IORING_TIMEOUT_ETIME_SUCCESS	(1U << 5)
>  #define IORING_TIMEOUT_MULTISHOT	(1U << 6)
> +#define IORING_TIMEOUT_IMMEDIATE_ARG	(1U << 7)
>  #define IORING_TIMEOUT_CLOCK_MASK	(IORING_TIMEOUT_BOOTTIME | IORING_TIMEOUT_REALTIME)
>  #define IORING_TIMEOUT_UPDATE_MASK	(IORING_TIMEOUT_UPDATE | IORING_LINK_TIMEOUT_UPDATE)
>  /*
> diff --git a/io_uring/timeout.c b/io_uring/timeout.c
> index d97f67d85ea3..e051c8374c1a 100644
> --- a/io_uring/timeout.c
> +++ b/io_uring/timeout.c
> @@ -528,7 +528,8 @@ static int __io_timeout_prep(struct io_kiocb *req,
>  	flags = READ_ONCE(sqe->timeout_flags);
>  	if (flags & ~(IORING_TIMEOUT_ABS | IORING_TIMEOUT_CLOCK_MASK |
>  		      IORING_TIMEOUT_ETIME_SUCCESS |
> -		      IORING_TIMEOUT_MULTISHOT))
> +		      IORING_TIMEOUT_MULTISHOT |
> +		      IORING_TIMEOUT_IMMEDIATE_ARG))
>  		return -EINVAL;
>  	/* more than one clock specified is invalid, obviously */
>  	if (hweight32(flags & IORING_TIMEOUT_CLOCK_MASK) > 1)
> @@ -557,8 +558,14 @@ static int __io_timeout_prep(struct io_kiocb *req,
>  	data->req = req;
>  	data->flags = flags;
>  
> -	if (get_timespec64(&data->ts, u64_to_user_ptr(READ_ONCE(sqe->addr))))
> +	if (flags & IORING_TIMEOUT_IMMEDIATE_ARG) {
> +		if (flags & IORING_TIMEOUT_ABS)
> +			return -EINVAL;
> +		data->ts = ns_to_timespec64(READ_ONCE(sqe->addr));
> +	} else if (get_timespec64(&data->ts,
> +				  u64_to_user_ptr(READ_ONCE(sqe->addr)))) {
>  		return -EFAULT;
> +	}
>  
>  	if (data->ts.tv_sec < 0 || data->ts.tv_nsec < 0)
>  		return -EINVAL;

Looks good to me on the feature side, makes sense. But like the 1/2
patch, this one needs to update the remove side as well to support the
immediate arg.

-- 
Jens Axboe