On Wed, May 22, 2019 at 09:20:52PM -0300, Jason Gunthorpe wrote:
On Wed, May 22, 2019 at 02:49:28PM +0100, Dave Martin wrote:
On Tue, May 21, 2019 at 03:48:56PM -0300, Jason Gunthorpe wrote:
On Fri, May 17, 2019 at 03:49:31PM +0100, Catalin Marinas wrote:
The tagged pointers (whether hwasan or MTE) should ideally be a transparent feature for the application writer but I don't think we can solve it entirely and make it seamless for the multitude of ioctls(). I'd say you only opt in to such feature if you know what you are doing and the user code takes care of specific cases like ioctl(), hence the prctl() proposal even for the hwasan.
I'm not sure such a dire view is warrented..
The ioctl situation is not so bad, other than a few special cases, most drivers just take a 'void __user *' and pass it as an argument to some function that accepts a 'void __user *'. sparse et al verify this.
As long as the core functions do the right thing the drivers will be OK.
The only place things get dicy is if someone casts to unsigned long (ie for vma work) but I think that reflects that our driver facing APIs for VMAs are compatible with static analysis (ie I have no earthly idea why get_user_pages() accepts an unsigned long), not that this is too hard.
If multiple people will care about this, perhaps we should try to annotate types more explicitly in SYSCALL_DEFINEx() and ABI data structures.
For example, we could have a couple of mutually exclusive modifiers
T __object * T __vaddr * (or U __vaddr)
In the first case the pointer points to an object (in the C sense) that the call may dereference but not use for any other purpose.
How would you use these two differently?
So far the kernel has worked that __user should tag any pointer that is from userspace and then you can't do anything with it until you transform it into a kernel something
Ultimately it would be good to disallow casting __object pointers execpt to compatible __object pointer types, and to make get_user etc. demand __object.
__vaddr pointers / addresses would be freely castable, but not to __object and so would not be dereferenceable even indirectly.
Or that's the general idea. Figuring out a sane set of rules that we could actually check / enforce would require a bit of work.
(Whether the __vaddr base type is a pointer or an integer type is probably moot, due to the restrictions we would place on the use of these anyway.)
to tell static analysers the real type of pointers smuggled through UAPI disguised as other types (*cough* KVM, etc.)
Yes, that would help alot, we often have to pass pointers through a u64 in the uAPI, and there is no static checker support to make sure they are run through the u64_to_user_ptr() helper.
Agreed.
Cheers ---Dave