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] [day] [month] [year] [list]

Message-ID: <9b6522bc-314e-d663-a035-c4614b21b756@kernel.dk>
Date:   Tue, 25 Jul 2023 15:24:30 -0600
From:   Jens Axboe <axboe@...nel.dk>
To:     Peter Zijlstra <peterz@...radead.org>
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 7/25/23 2:42?PM, Jens Axboe wrote:
> On 7/25/23 9:19?AM, Peter Zijlstra wrote:
>> On Tue, Jul 25, 2023 at 07:57:28AM -0600, Jens Axboe wrote:
>>
>>> Something like the below - totally untested, but just to show what I
>>> mean. Will need to get split and folded into the two separate patches.
>>> Will test and fold them later today.
>>>
>>>
>>> diff --git a/io_uring/futex.c b/io_uring/futex.c
>>> index 4c9f2c841b98..b0f90154d974 100644
>>> --- a/io_uring/futex.c
>>> +++ b/io_uring/futex.c
>>> @@ -168,7 +168,7 @@ bool io_futex_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
>>>  	return found;
>>>  }
>>>  
>>> -int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>> +static 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);
>>>  	u32 flags;
>>> @@ -179,9 +179,6 @@ int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>  	iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
>>>  	iof->futex_val = READ_ONCE(sqe->addr2);
>>>  	iof->futex_mask = READ_ONCE(sqe->addr3);
>>> -	iof->futex_nr = READ_ONCE(sqe->len);
>>> -	if (iof->futex_nr && req->opcode != IORING_OP_FUTEX_WAITV)
>>> -		return -EINVAL;
>>>  
>>>  	flags = READ_ONCE(sqe->futex_flags);
>>>  	if (flags & ~FUTEX2_MASK)
>>> @@ -191,14 +188,36 @@ int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>  	if (!futex_flags_valid(iof->futex_flags))
>>>  		return -EINVAL;
>>>  
>>> -	if (!futex_validate_input(iof->futex_flags, iof->futex_val) ||
>>> -	    !futex_validate_input(iof->futex_flags, iof->futex_mask))
>>> +	if (!futex_validate_input(iof->futex_flags, iof->futex_mask))
>>>  		return -EINVAL;
>>>  
>>> -	iof->futexv_owned = 0;
>>>  	return 0;
>>>  }
>>
>> I think you can/should split more into io_futex_prep(), specifically
>> waitv should also have zero @val and @mask.
> 
> Yep, I'll include that. Updating them now...

It ends up just being this incremental for the very last patch, moving
all the waitv related prep to the wait prep and not relying on the
non-vectored one at all.


diff --git a/io_uring/futex.c b/io_uring/futex.c
index 4c9f2c841b98..e885aac12df8 100644
--- a/io_uring/futex.c
+++ b/io_uring/futex.c
@@ -179,9 +179,6 @@ int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	iof->futex_val = READ_ONCE(sqe->addr2);
 	iof->futex_mask = READ_ONCE(sqe->addr3);
-	iof->futex_nr = READ_ONCE(sqe->len);
-	if (iof->futex_nr && req->opcode != IORING_OP_FUTEX_WAITV)
-		return -EINVAL;
 
 	flags = READ_ONCE(sqe->futex_flags);
 	if (flags & ~FUTEX2_MASK)
@@ -195,7 +192,6 @@ int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	    !futex_validate_input(iof->futex_flags, iof->futex_mask))
 		return -EINVAL;
 
-	iof->futexv_owned = 0;
 	return 0;
 }
 
@@ -220,10 +216,13 @@ int io_futexv_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	struct futex_vector *futexv;
 	int ret;
 
-	ret = io_futex_prep(req, sqe);
-	if (ret)
-		return ret;
+	/* No flags or mask supported for waitv */
+	if (unlikely(sqe->fd || sqe->buf_index || sqe->file_index ||
+		     sqe->addr2 || sqe->addr3))
+		return -EINVAL;
 
+	iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
+	iof->futex_nr = READ_ONCE(sqe->len);
 	if (!iof->futex_nr || iof->futex_nr > FUTEX_WAITV_MAX)
 		return -EINVAL;
 
@@ -238,6 +237,7 @@ int io_futexv_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		return ret;
 	}
 
+	iof->futexv_owned = 0;
 	req->flags |= REQ_F_ASYNC_DATA;
 	req->async_data = futexv;
 	return 0;

-- 
Jens Axboe