linux-kernel - RE: [PATCH] net: untag pointer in sockptr_is

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] [day] [month] [year] [list]

Message-ID: <df130026160145f1abc1cbd63f11e8d3@AcuMS.aculab.com>
Date:   Wed, 12 Aug 2020 09:48:48 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Miles Chen' <miles.chen@...iatek.com>
CC:     'Christoph Hellwig' <hch@....de>,
        "David S . Miller" <davem@...emloft.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "wsd_upstream@...iatek.com" <wsd_upstream@...iatek.com>
Subject: RE: [PATCH] net: untag pointer in sockptr_is_kernel

From: Miles Chen <miles.chen@...iatek.com>
> Sent: 12 August 2020 10:16
> 
> On Tue, 2020-08-11 at 11:44 +0000, David Laight wrote:
> > > On Tue, Aug 11, 2020 at 06:27:04PM +0800, Miles Chen wrote:
> > > > From: Miles Chen <miles.chen@...iatek.com>
> > > >
> > > > sockptr_is_kernel() uses (sockptr.kernel >= TASK_SIZE) to tell
> > > > if the pointer is kernel space or user space. When user space uses
> > > > the "top byte ignored" feature such as HWAsan, we must untag
> > > > the pointer before checking against TASK_SIZE.
> > > >
> > > > sockptr_is_kernel() will view a tagged user pointer as a kernel pointer
> > > > and use memcpy directly and causes a kernel crash.
> > >
> > > Dave merged a patch from me to rever the optimized sockptr
> > > implementation for now.  If we bring it back we should fold in your
> > > fix.
> >
> > I missed that going though :-(
> > I've looked for a fix to the access_ok(kernel_addr,0) being true issue.
> >
> > Shouldn't TASK_SIZE be increased to cover all the 'tagged' addresses?
> > ISTR the 'tag' bits are the 'next' 8 (or so) address bits at the top
> > of valid user addresses.
> 
> 
> I'm not sure if this is a good idea. TASK_SIZE is an arch dependent
> constant, if we increase TASK_SIZE to cover the 'tagged' address space,
> the TASK_SIZE will not tell us the actual virtual address size.
> 
> Maybe we need a macro to tell if a pointer is in user space or not and
> handle the memory tag there.
> But this only works for the "is this pointer in user space" problem.

Well TASK_SIZE isn't a constant, it is read out of 'current'.
Typically it isn't even the limit on the address space since
(at least in x86) it is changed by setfs(KERNEL_DS) so that
access_ok() doesn't reject kernel addresses.

It is almost as if TASK_SIZE is only actually valid for the
check in access_ok().

I'd also have thought you'd want the kernel to use the 'tagged'
address so that the hardware checks it matches.
Or is there some scheme for having userpace use tagged pointers
without hardware support - but wouldn't that require masking
and/or large offsets all over the user code?

	David

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