Re: [v2] serial_core:recognize invalid pointer from userspace

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2016年03月10日 21:46, One Thousand Gnomes wrote:
When userspace get setting with TIOCGSERIAL,

1.On 64bit kernel + 32bit rootfs, compat ioctl code use 0xffffffff to
mark invalid conversion.
Start at the beginning. What is being passed back and forth that causes
the problem. What memory address does your uart have ?

In fs/compat_ioctl.c, serial_struct_ioctl() use following code for convert a 64bit pointer into 32bit userspace pointer

        if (cmd == TIOCGSERIAL && err >= 0) {
                if (copy_in_user(ss32, ss, offsetof(SS32, iomem_base)) ||
                    get_user(iomem_base, &ss->iomem_base))
                        return -EFAULT;
                base = (unsigned long)iomem_base  >> 32 ?
                        0xffffffff : (unsigned)(unsigned long)iomem_base;

If iomem_base exceed 4G, TIOCGSERIAL didn't return right value for iomem_base.

it  use following code to covert 32bit to 64bit.
        if (cmd == TIOCSSERIAL) {
                if (copy_in_user(ss, ss32, offsetof(SS32, iomem_base)) ||
                    get_user(udata, &ss32->iomem_base))
                        return -EFAULT;
                iomem_base = compat_ptr(udata);


static inline void __user *compat_ptr(compat_uptr_t uptr)
{
        return (void __user *)(unsigned long)uptr;
}

You can see userspace application didn't get valid iomem_base via TIOCGSERAIL. When issuing TIOCSSERIAL, application can not provide a workable value for kernel. It means TIOCSSERIAL can not work.

Typical serial setting application setserial is depend on those 2 ioctls.


2.On 32bit kernel with 64bit physical address, uart_get_info() in
serial_core will truncate a 64bit address into 32bit pointer with
following code:
      retinfo->iomem_base      = (void *)(unsigned long)uport->mapbase;

Then when setting with TIOCSSERIAL ioctl, application can not provide
original value for iomem_base, it leads setserial can not work on such
boards.
Yes - it's a legacy feature that isn't really needed on 64bit systems.

I don't know why kernel should expose this value to userspace, and seems
no need for userspace application to update an physical address for driver.
Can we consider drop this member in handle for TIOCSSERIAL ioctl. Please
see a rough patch in attachment.
There are old PC and embedded cases from the time before devicetree and
ACPI were the default. It's now an API people rely upon.

It would make sense I think to return 0 for the base address if it
exceeds 32bits, and also to ignore a TIOCSSERIAL that passes 0 as the
address back. Would that fix your use case ?

If boards relay on this TIOCSSERIAL only working on 32bit, this solution is workable.


Thanks
Jiang Lu




Alan


--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux