Re: <Query> Looking more details and reasons for using orig_add_limit.
From: Sodagudi Prasad
Date: Wed Feb 15 2017 - 16:12:17 EST
Hi James and Will,
Thanks James and Will for providing detailed information.
On 2017-02-15 04:09, James Morse wrote:
On 15/02/17 05:52, Sodagudi Prasad wrote:
When any sys call is made from user space orig_addr_limit will be zero
that driver is calling set_fs(KERNEL_DS) and then copy_to_user() to
Don't do this, its exactly the case PAN+UAO and the code you pointed to
designed to catch. Accessing userspace needs doing carefully, setting
and using the put_user()/copy_to_user() accessors are the required
Which driver is doing this? Is it in mainline?
Yes. It is mainline driver -
Currently working on a platform which is ARMv8 based, so disabled
and ARMv8.2 features (ARM64_PAN, ARM64_HW_AFDBM, LSE_ATOMICS and
on lsk-v4.4-16.09. In some v4l2 use-case kernel panic is observed. Below
of the code has set_fs to KERNEL_DS before calling native_ioctl().
static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned
err = native_ioctl(file, cmd, (unsigned long)up);
mm_segment_t old_fs = get_fs();
set_fs(KERNEL_DS); ====> KERNEL_DS.
err = native_ioctl(file, cmd, (unsigned long)&karg);
Here is the call stack which is resulting crash, because user space
read only permissions.
[27249.920041] [<ffffff8008357890>] __arch_copy_to_user+0x110/0x180
[27249.920047] [<ffffff8008847c98>] video_ioctl2+0x38/0x44
[27249.920054] [<ffffff8008840968>] v4l2_ioctl+0x78/0xb4
[27249.920059] [<ffffff80088542d8>] do_video_ioctl+0x91c/0x1160
[27249.920064] [<ffffff8008854b7c>] v4l2_compat_ioctl32+0x60/0xcc
[27249.920071] [<ffffff800822553c>] compat_SyS_ioctl+0x124/0xd88
[27249.920077] [<ffffff8008084e30>] el0_svc_naked+0x24/0x2
If there is permission fault for user space address the above
is leading to kernel crash. Because orig_add_limit is having KERNEL_DS
called before copy_to_user().
1) So I would like to understand that, is that user space pointer
permission fault not correct(condition_1) in this scenario?
The correct thing has happened here. To access user space
(and set it back to whatever it was afterwards).
So, Any clean up needed to above call path similar to what was done in
the below commit?
commit a7f61e89af73e9bf760826b20dba4e637221fcb9 - compat_ioctl: don't
call do_ioctl under set_fs(KERNEL_DS)
2) Are there any corner cases where these if conditions
condition_2) would lead to kernel crash ?
If you do this on behalf of a user space process the kernel will try to
as best it can and carry on. If you access user space from an interrupt
or from a kernel thread you can expect the kernel to panic().
3) What are all scenarios these if conditions (condition_1 and
would like to take care?
I'm not sure I understand this question. PAN prevents general kernel
accessing user space, you have to use the accessors. When you have UAO
can enforce the set_fs() limit as PAN will generate permission faults
accessors touch the kernel/user-space after setting the other set_fs()
I hope this helps!
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Linux Foundation Collaborative Project