linux-kernel - Re: [PATCH 06/10] io_uring: add support for futex wake and wait

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230721113031.GG3630545@hirez.programming.kicks-ass.net>
Date:   Fri, 21 Jul 2023 13:30:31 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Jens Axboe <axboe@...nel.dk>
Cc:     io-uring@...r.kernel.org, linux-kernel@...r.kernel.org,
        andres@...razel.de
Subject: Re: [PATCH 06/10] io_uring: add support for futex wake and wait

On Thu, Jul 20, 2023 at 04:18:54PM -0600, Jens Axboe wrote:


> +struct io_futex {
> +	struct file	*file;
> +	u32 __user	*uaddr;
> +	unsigned int	futex_val;
> +	unsigned int	futex_flags;
> +	unsigned int	futex_mask;
> +};

So in the futex patches I just posted I went with 'unsigned long'
(syscall) or 'u64' (data structures) for the futex, such that, on 64bit
platforms, we might support 64bit futexes in the future (I still need to
audit the whole futex internals and convert u32 to unsigned long in
order to enable that).

So would something like:

struct io_futex {
	struct file	*file;
	void __user	*uaddr;
	u64		futex_val;
	u64		futex_mask;
	u32		futex_flags;
};

work to match the futex2 syscalls?



> +int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> +{
> +	struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
> +
> +	if (unlikely(sqe->fd || sqe->addr2 || sqe->buf_index || sqe->addr3))
> +		return -EINVAL;
> +
> +	iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
> +	iof->futex_val = READ_ONCE(sqe->len);
> +	iof->futex_mask = READ_ONCE(sqe->file_index);
> +	iof->futex_flags = READ_ONCE(sqe->futex_flags);

sqe->addr,		u64
sqe->len,		u32
sqe->file_index,	u32
sqe->futex_flags,	u32

> +	if (iof->futex_flags & FUTEX_CMD_MASK)

		FUTEX2_MASK

(which would need lifting from syscall.c to kernel/futex/futex.h I
suppose)

> +		return -EINVAL;
> +
> +	return 0;
> +}

> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index 36f9c73082de..3bd2d765f593 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -65,6 +65,7 @@ struct io_uring_sqe {
>  		__u32		xattr_flags;
>  		__u32		msg_ring_flags;
>  		__u32		uring_cmd_flags;
> +		__u32		futex_flags;
>  	};
>  	__u64	user_data;	/* data to be passed back at completion time */
>  	/* pack this to avoid bogus arm OABI complaints */

Perhaps extend it like so?


diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 08720c7bd92f..c1d28bf64d11 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -35,6 +35,7 @@ struct io_uring_sqe {
 	union {
 		__u64	off;	/* offset into file */
 		__u64	addr2;
+		__u64	futex_val;
 		struct {
 			__u32	cmd_op;
 			__u32	__pad1;
@@ -65,6 +66,7 @@ struct io_uring_sqe {
 		__u32		xattr_flags;
 		__u32		msg_ring_flags;
 		__u32		uring_cmd_flags;
+		__u32		futex_flags;
 	};
 	__u64	user_data;	/* data to be passed back at completion time */
 	/* pack this to avoid bogus arm OABI complaints */
@@ -87,6 +89,7 @@ struct io_uring_sqe {
 	union {
 		struct {
 			__u64	addr3;
+			__u64	futex_mask;
 			__u64	__pad2[1];
 		};
 		/*


So that we can write something roughtly like:

	iof->uaddr = sqe->addr;
	iof->val   = sqe->futex_val;
	iof->mask  = sqe->futex_mask;
	iof->flags = sqe->futex_flags;

	if (iof->flags & ~FUTEX2_MASK)
		return -EINVAL;