linux-kernel - RE: [PATCH -next] uaccess: fix __access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bdebdcb56b8a4af8a6b1d22029a2e7ba@AcuMS.aculab.com>
Date:   Wed, 23 Mar 2022 01:35:34 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Arnd Bergmann' <arnd@...db.de>,
        "chenjiahao (C)" <chenjiahao16@...wei.com>
CC:     "Eric W . Biederman" <ebiederm@...ssion.com>,
        Sam Ravnborg <sam@...nborg.org>,
        Stafford Horne <shorne@...il.com>,
        Dinh Nguyen <dinguyen@...nel.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        "Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
        Linux API <linux-api@...r.kernel.org>,
        Linux FS-devel Mailing List <linux-fsdevel@...r.kernel.org>,
        the arch/x86 maintainers <x86@...nel.org>
Subject: RE: [PATCH -next] uaccess: fix __access_ok limit setup in compat mode

From: Arnd Bergmann
> Sent: 22 March 2022 14:41
> 
> On Tue, Mar 22, 2022 at 1:55 PM chenjiahao (C) <chenjiahao16@...wei.com> 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)