Re: i386: kernel overflow in syscall

[David Laight <david%l8s.co.uk@localhost>]

On Sun, Sep 11, 2016 at 11:51:27AM +0200, Maxime Villard wrote:
> When syscalling on i386, the kernel copies in the sycall args with a special
> function [1]. The third argument is the size of the userland area. In
> x86_copyargs, however, you can see [2] that the area is copied by two chunks
> of 16 and 20 bytes, which means that if a syscall has a 4-byte-sized
> argument, the kernel still copies 16 bytes.
> 
> The problem is that the VM_MAXUSER_ADDRESS check (in charge of making sure
> we don't start reading kernel space) is done on the third argument, and not
> the real copied size.
> 
> It is easy to create a condition where %esp+4+arg_size <= VM_MAXUSER_ADDRES
> and %esp+4+roundup(arg_size, 16) > VM_MAXUSER_ADDRESS. In such a case, the
> userland check succeeds, and the kernel starts copying its own memory.
> 
> Without going into terrible details, it is possible for userland to make
> sure VM_MAXUSER_ADDRESS+delta is readable, and therefore, x86_copyargs will
> succeed, and some args will contain some slots of the page tree.
> 
> This bug seems harmless, but I guess it is still possible to leak kernel
> data to userland. I wrote [3] quickly, which triggers such a condition and
> goes unseen.
> 
> x86_copyargs is supposed to be an 'optimized' copyin. While the design of
> this function is obviously wrong, I would simply suggest rounding up the
> size of the third argument [4].
> 
> Just sayin
> 
> [1] https://nxr.netbsd.org/xref/src/sys/arch/x86/x86/syscall.c#150
> [2] https://nxr.netbsd.org/xref/src/sys/arch/i386/i386/copy.S#679
> [3] http://m00nbsd.net/garbage/sysover/sysover.c
> [4] http://m00nbsd.net/garbage/sysover/syscall.diff

It was completely harmless.
The values being read are the system call parameters, the length
check was against the number of parameters the kernel code would
look at.
So the extra values should never even be looked at - never mind
leaked back to userspace.
Patch [4] was completely wrong - it increased the size of the copy.
Checking the max size (as commited) is ok though.
Can save an instruction be using lea.

	David

-- 
David Laight: david%l8s.co.uk@localhost