RE: [PATCH] net: untag pointer in sockptr_is_kernel

[David Laight]

From: Miles Chen <miles.chen@xxxxxxxxxxxx>
> 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@xxxxxxxxxxxx>
> > > >
> > > > 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)