[PATCH] Adds a new ioctl32 syscall for backwards compatibility layers

[Arnd Bergmann]

On Fri, Jan 15, 2021 at 3:06 AM Ryan Houdek <sonicadvance1 at gmail.com> wrote:
> On Wed, Jan 6, 2021 at 12:49 AM Arnd Bergmann <arnd at kernel.org> wrote:
>> On Wed, Jan 6, 2021 at 7:48 AM <sonicadvance1 at gmail.com> wrote:
>> > From: Ryan Houdek <Sonicadvance1 at gmail.com>
>> ...
>>
>> 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

>> > 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.

>> >  }
>> > +
>> > +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