RE: [PATCH -next] uaccess: fix __access_ok limit setup in compat mode

[David Laight]

From: Arnd Bergmann
> Sent: 22 March 2022 14:41
> 
> On Tue, Mar 22, 2022 at 1:55 PM chenjiahao (C) <chenjiahao16@xxxxxxxxxx> wrote:
> > 在 2022/3/18 15:44, Arnd Bergmann 写道:
> > >
> > > This should not result in any user visible difference, in both cases
> > > user process will see a -EFAULT return code from its system call.
> > > Are you able to come up with a test case that shows an observable
> > > difference in behavior?
> >
> > Actually, this patch do comes from a testcase failure, the code is
> > pasted below:
> 
> Thank you for the test case!
> 
...
> >      ret = pread64(fd, buf, -1, 1);
> >      if((-1 == ret) && (EFAULT == errno))
> >      {
...
> >          printf("PASS\n");
...
> > In my explanation, pread64 is called with count '0xffffffffull' and
> > offset '1', which might still not trigger
> >
> > page fault in 64-bit kernel.
> >
> >
> > This patch uses TASK_SIZE as the addr_limit to performance a stricter
> > address check and intercepts
> 
> I see. So while the kernel behavior was not meant to change from
> my patch, it clearly did, which may cause problems. However, I'm
> not sure if the changed behavior is actually wrong.

It isn't really any different from passing a length of (1 << 30)
(and a buffer at a low user address).
The entire buffer is valid user addresses, but most of it is
invalid because there is nothing mapped at the relevant addresses.
Unless you actually try to access one of the memory locations
you won't get a fault - and the correct return is then a partial read.

Similarly it is valid for the kernel to ensure there is an
unmapped page between user and kernel addresses and then
not check the buffer size at all - requiring the kernel code
do (adequately) sequential accesses.
Again your test 'fails'.

You could equally well argue that the 'old' behaviour is wrong!

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)