On 15/06/2023 17:58, Tudor Cretu wrote:
-typedef __kernel_ulong_t aio_context_t; +typedef __kernel_uintptr_t aio_context_t;
__kernel_uintptr_t is only backwards-compatible when replacing __u64. In this case we would effectively expand the type to 64 bits in 32-bit, which is not intended.
I thought we can get away with this given that io_setup has a compat handler and would return a 32-bit truncated address anyway...
compat is not the issue, we already have compat_aio_context_t anyway. The issue is 32-bit architectures: making aio_context_t 64-bit on them wouldn't be backwards-compatible.
I wonder if we could get away with just defining it as void __user *. It looks like a few fixups in the kernel would be needed (Clang and GCC don't seem to understand T __capability * if T is itself a capability), and more importantly it is a uapi change, but I am quite hopeful it wouldn't break much if anything. libaio doesn't use aio_context_t and instead defines its own io_context_t as a pointer. A Debian Code Search does show quite a few users of aio_context_t, but AFAICT it is only ever passed to the AIO syscalls or initialised to 0 (works if defined as a pointer too) - pretty much an opaque type.
I also thought about changing it to void __user *. It's a bit odd having the compat_aio_context_t be an integer value and this to be a pointer... I separated the aio_context_t in another patch, for clarity around all the changes. Feel free to suggest as many improvements as you see fit, I'm dedicated to make this series as neat and proper as it can be. :)
Very nice change indeed! For compat_aio_context_t, it's not really weird: all compat types are integers, even if they represent pointers. That said you make a good point, since aio_context_t is now an actual user pointer, we could make compat_aio_context_t a compat_uptr_t to be consistent (also avoids having a different definition in compat64)..
Kevin