On Fri, Jan 15, 2021 at 3:06 AM Ryan Houdek [off-list ref] wrote:

On Wed, Jan 6, 2021 at 12:49 AM Arnd Bergmann [off-list ref] wrote:

quoted

On Wed, Jan 6, 2021 at 7:48 AM [off-list ref] wrote:

quoted

From: Ryan Houdek <redacted>

...

For x86, this has another complication, as some ioctls also need to
check whether they are in an ia32 task (with packed u64 and 32-bit
__kernel_old_time_t) or an x32 task (with aligned u64 and 64-bit
__kernel_old_time_t). If the new syscall gets wired up on x86 as well,
you'd need to decide which of the two behaviors you want.

I can have a follow-up patch that makes this do ni-syscall on x86_64 since
we can go through the int 0x80 handler, or x32 handler path and choose
whichever one there.

I'd say for consistency

quoted

quoted

3a) Userspace consumes all VA space above 32bit. Forcing allocations to
occur in lower 32bits
  - This is the current implementation
3b) Ensure any allocation in the ioctl handles ioctl entrypoint rather
than just allow generic memory allocations in full VA space
  - This is hard to guarantee

What kind of allocation do you mean here? Can you give an example of
an ioctl that does this?

My concern here would be something like DRM allocating memory and
returning a pointer to userspace that ends up in 64bit space.
I can see something like `drm_get_unmapped_area` calls in to
 `current->mm->get_unmapped_area` which I believe only ends up falling
down TASK_SIZE checks.

I see.

Which could potentially return pointers in the 64bit address space range
in this case. Theoretically can be resolved either by thieving the full 64bit
VA range, or doing something like the Tango layer patches that on
syscall entry changes the syscall to a "compat" syscall.
compat syscall flag like Tango might be nicer here?

Not sure how that flag is best encoded, but yes, it would have to be
somewhere that arch_get_unmapped_area() and
arch_get_mmap_end() can find. Clearly we want a solution that works
for both tango and for your work, as well as being portable to any
architecture.

quoted

quoted

 }
+
+COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd,
+                       compat_ulong_t, arg)
+{
+       return do_ioctl32(fd, cmd, arg);
+}
+
+SYSCALL_DEFINE3(ioctl32, unsigned int, fd, unsigned int, cmd,
+                       compat_ulong_t, arg)
+{
+       return do_ioctl32(fd, cmd, arg);
+}

These two look identical to me, I don't think you need to add a wrapper
here at all, but can just use the normal compat_sys_ioctl entry point
unless you want to add a 'flags' argument to control the struct padding.

I tried having the dispatch table call directly in to the COMPAT one and
the way things were lining up weren't allowing me to do this.
Since this is a bit unique in how it operates, I'm not quite sure if there is
another example I could pull from for this.

For the asm-generic/unistd.h, you should be able to write htis as

#if __BITS_PER_LONG == 64
__SC_COMP(__NR_ioctl32, compat_sys_ioctl, sys_ni_syscall)
#endif

Which means that the native syscall in a 64-bit process always
points to compat_sys_ioctl, while a 32-bit process always gets
-ENOSYS.

Similarly, the syscall_64.tbl file on x86 and the other 64-bit
architectures would use
442    64      ioctl32         compat_sys_ioctl

FWIW, I suppose you can rename compat_sys_ioctl to
sys_ioctl32 treewide, if that name makes more sense.

      Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel