On 15/06/2023 17:54, Tudor Cretu wrote:
The libaio library may set up I/O completion callback functions for I/O requests. The callback function pointer is stored in the data field of the io_event struct, and the pointer to the original iocb struct is stored in the obj field. Therefore, both the data and obj fields of the io_event struct might hold user pointers.
In the PCuABI, a user pointer is a 129-bit capability, so the __u64 type is not big enough to hold it. Use the __kernel_uintptr_t type instead, which is big enough on the affected architectures while remaining 64-bit on others.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com
fs/aio.c | 20 ++++++++++---------- include/uapi/linux/aio_abi.h | 10 +++++----- 2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c index dff01598760e..cb0b5de179df 100644 --- a/fs/aio.c +++ b/fs/aio.c
Surely in copy_io_events_to_user() we need to use copy_to_user_with_ptr()?
@@ -344,8 +344,8 @@ static void copy_io_event_to_ring(struct kioctx *ctx, if (aio_in_compat64(ctx)) { struct compat_io_event *compat_ring_event = &ctx->io_events_compat[ring_tail];
compat_ring_event->data = native_event->data;compat_ring_event->obj = native_event->obj;
compat_ring_event->data = (__u64)native_event->data;compat_ring_event->obj = (__u64)native_event->obj;@@ -1229,9 +1229,10 @@ static void aio_complete(struct aio_kiocb *iocb) copy_io_event_to_ring(ctx, tail, &iocb->ki_res);
- pr_debug("%p[%u]: %p: %p %Lx %Lx %Lx\n", ctx, tail, iocb,
(void __user *)(unsigned long)iocb->ki_res.obj,iocb->ki_res.data, iocb->ki_res.res, iocb->ki_res.res2);
- pr_debug("%p[%u]: %p: %Lx %Lx %Lx %Lx\n", ctx, tail, iocb,
(unsigned long long)iocb->ki_res.obj,(unsigned long long)iocb->ki_res.data,iocb->ki_res.res, iocb->ki_res.res2);/* after flagging the request as done, we * must never even look at it again @@ -2048,7 +2049,7 @@ static int __io_submit_one(struct kioctx *ctx, const struct iocb *iocb, return -EFAULT; }
- req->ki_res.obj = user_ptr_addr(user_iocb);
- req->ki_res.obj = (__kernel_uintptr_t)user_iocb; req->ki_res.data = iocb->aio_data; req->ki_res.res = 0; req->ki_res.res2 = 0;
@@ -2079,7 +2080,7 @@ static int get_compat_iocb(struct iocb *iocb, const struct iocb __user *user_ioc struct compat_iocb compat_iocb; if (unlikely(copy_from_user(&compat_iocb, user_iocb, sizeof(struct compat_iocb)))) return -EFAULT;
- iocb->aio_data = compat_iocb.aio_data;
- iocb->aio_data = (__kernel_uintptr_t)compat_iocb.aio_data; iocb->aio_key = compat_iocb.aio_key; iocb->aio_rw_flags = compat_iocb.aio_rw_flags; iocb->aio_lio_opcode = compat_iocb.aio_lio_opcode;
@@ -2263,7 +2264,6 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb, struct aio_kiocb *kiocb; int ret = -EINVAL; u32 key;
- u64 obj = user_ptr_addr(iocb);
if (in_compat_syscall()) ctx_id = compat_ptr(user_ptr_addr(ctx_id)); @@ -2285,7 +2285,7 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb, spin_lock_irq(&ctx->ctx_lock); /* TODO: use a hash or array, this sucks. */ list_for_each_entry(kiocb, &ctx->active_reqs, ki_list) {
if (kiocb->ki_res.obj == obj) {
if (user_ptr_is_same((struct iocb __user *)kiocb->ki_res.obj, iocb)) { ret = kiocb->ki_cancel(&kiocb->rw); list_del_init(&kiocb->ki_list); break;@@ -2385,7 +2385,7 @@ SYSCALL_DEFINE6(io_pgetevents, if (timeout && unlikely(get_timespec64(&ts, timeout))) return -EFAULT;
- if (usig && copy_from_user(&ksig, usig, sizeof(ksig)))
- if (usig && copy_from_user_with_ptr(&ksig, usig, sizeof(ksig)))
Arguably this is an unrelated fix (i.e. it was already broken before this series and we would have found out if uaccess was checked), so would be better as a separate patch.
Kevin
return -EFAULT;ret = set_user_sigmask(ksig.sigmask, ksig.sigsetsize); diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h index 87c270021449..edd2ac54ac42 100644 --- a/include/uapi/linux/aio_abi.h +++ b/include/uapi/linux/aio_abi.h @@ -58,10 +58,10 @@ enum { /* read() from /dev/aio returns these structures. */ struct io_event {
- __u64 data; /* the data field from the iocb */
- __u64 obj; /* what iocb this event came from */
- __s64 res; /* result code for this event */
- __s64 res2; /* secondary result */
- __kernel_uintptr_t data; /* the data field from the iocb */
- __kernel_uintptr_t obj; /* what iocb this event came from */
- __s64 res; /* result code for this event */
- __s64 res2; /* secondary result */
}; /* @@ -72,7 +72,7 @@ struct io_event { struct iocb { /* these are internal to the kernel/libc. */
- __u64 aio_data; /* data to be returned in event's data */
- __kernel_uintptr_t aio_data; /* data to be returned in event's data */
#if defined(__BYTE_ORDER) ? __BYTE_ORDER == __LITTLE_ENDIAN : defined(__LITTLE_ENDIAN) __u32 aio_key; /* the kernel sets aio_key to the req # */