Update operations truncate the provided user_data to 32bit on 32bit builds.

[rbernon]

The internal structures use __u64 both for addr field and for user_data fields, which is nice as they are usually matched against each other for update operations.

However, the internal io_uring_prep_rw function takes a void * addr, (but then casts it back to __u64), Several other public API functions, are doing truncation, such as io_uring_prep_poll_update, which takes void* old_user_data / void* new_user_data, or even worse, io_uring_prep_timeout_update which takes a __u64 user_data to match the timeout that needs updating but internally casts it to unsigned long / void * when calling io_uring_prep_rw.

Overall, this makes the liburing API fairly unusable on 32bit for such operations, as the addr field get truncated, and the user needs to fill the sqe fields by hand. This is pretty inconvenient for writing portable code, and the API should make sure no unnecessary truncation happens.

[axboe]

What cases on 32-bit wants to put a 64-bit address into what should be a user pointer? Looking at this more closely, I'm a bit puzzled on what commands are involved here? Off the top of my head I don't recall any commands actually not using that for an actual user address, in which case how could that even work to put a non-valid 32-bit pointer in there?

Is this just a timeout command (and/or update) issue? Are there others? From a quick look, looks like timeout_update and timeout_remove are the only problematic ones here.

[axboe]

And if so, why don't we just do:

diff --git a/src/include/liburing.h b/src/include/liburing.h
index 169e0983a276..230cd6195689 100644
--- a/src/include/liburing.h
+++ b/src/include/liburing.h
@@ -425,8 +425,8 @@ static inline void io_uring_prep_timeout(struct io_uring_sqe *sqe,
 static inline void io_uring_prep_timeout_remove(struct io_uring_sqe *sqe,
 						__u64 user_data, unsigned flags)
 {
-	io_uring_prep_rw(IORING_OP_TIMEOUT_REMOVE, sqe, -1,
-				(void *)(unsigned long)user_data, 0, 0);
+	io_uring_prep_rw(IORING_OP_TIMEOUT_REMOVE, sqe, -1, NULL, 0, 0);
+	sqe->addr = user_data;
 	sqe->timeout_flags = flags;
 }
 
@@ -434,9 +434,9 @@ static inline void io_uring_prep_timeout_update(struct io_uring_sqe *sqe,
 						struct __kernel_timespec *ts,
 						__u64 user_data, unsigned flags)
 {
-	io_uring_prep_rw(IORING_OP_TIMEOUT_REMOVE, sqe, -1,
-				(void *)(unsigned long)user_data, 0,
-				(uintptr_t)ts);
+	io_uring_prep_rw(IORING_OP_TIMEOUT_REMOVE, sqe, -1, NULL, 0,
+				(uintptr_t) ts);
+	sqe->addr = user_data;
 	sqe->timeout_flags = flags | IORING_TIMEOUT_UPDATE;
 }

and avoid any kind of breakage related to this.

[rbernon]

Well the documentation says that, io_uring_prep_timeout_update, io_uring_prep_timeout_remove, io_uring_prep_poll_update, io_uring_prep_poll_remove, io_uring_prep_cancel, all target a previously submitted sqe with a matching user_data, but they all take a void * to match it, which isn't correct as the match is going to happen on the 64bit value.

The previously submitted sqe user_data may be any 64bit value, including on 32bit as the submitter may have set the value itself, without going through the io_uring_sqe_set_data helpers. For instance if you only need an index (or fd) and operation type, which fit well on 64bits, you don't really want to use a pointer there.

[axboe]

Yeah, you are right, there are a few more. I'll take a look at those.