Re another alternative to resolve hardened user copy warnings

Re another alternative to resolve hardened user copy warnings

[Bob Pearson]

Currently only the rxe driver is exhibiting the issue of kernel warnings during qp create caused by recent kernel changes looking for potential information leaks to user space. The test which triggers this warning is very specific. It occurs when a portion of a kernel object stored in a slab cache is copied to user space and the copied area has not been 'whitelisted' by setting useroffset and usersize parameters for the kmem cache. As already discussed there are two ways to mitigate this

	- copy the data, in this case an int, out of the kmem cache object then copy that to user space
	- play by the rules and set useroffset and usersize using kmem_cache_create_usercopy()

Additionally, as you have mentioned, we could move the allocation to the core (like AH, CQ, PD, etc.). But this only works for a different reason. If you use kmalloc or kzalloc then the warning is not triggered either and core uses these to allocate objects.

Allocating memory from kmalloc or kzalloc is effectively the same as kmem cache except that the pre-allocated sizes are typically powers of 2 so there is some wastage of memory and you have the potential to pre initialize all the objects in the kmem cache (e.g. locks, constants, etc) saving some time. Currently rxe does not take advantage of this second feature because resource allocation is not on the performance path for RDMA. Given this we have a third alternative which is

	- get rid of the kmem caches and use kzalloc to allocate storage for objects in the pools.

This is a simple change to rxe_pool.c, easy to understand, little or no change to the rest of the driver. This does not prevent moving other object allocations into the core but will resolve the warning issue.

Bob

Re: Re another alternative to resolve hardened user copy warnings

[Jason Gunthorpe]

On Tue, Aug 25, 2020 at 11:37:52AM -0500, Bob Pearson wrote:

> Currently only the rxe driver is exhibiting the issue of kernel
> warnings during qp create caused by recent kernel changes looking
> for potential information leaks to user space. The test which
> triggers this warning is very specific. It occurs when a portion of
> a kernel object stored in a slab cache is copied to user space and
> the copied area has not been 'whitelisted' by setting useroffset and
> usersize parameters for the kmem cache. As already discussed there
> are two ways to mitigate this

I think we should just add a uverbs_copy_to() for integers, much like
netlink does.

We already have various getters for integers..

Jason

Re: Re another alternative to resolve hardened user copy warnings

[Bob Pearson]

On 8/25/20 11:53 AM, Jason Gunthorpe wrote:
> On Tue, Aug 25, 2020 at 11:37:52AM -0500, Bob Pearson wrote:
> 
>> Currently only the rxe driver is exhibiting the issue of kernel
>> warnings during qp create caused by recent kernel changes looking
>> for potential information leaks to user space. The test which
>> triggers this warning is very specific. It occurs when a portion of
>> a kernel object stored in a slab cache is copied to user space and
>> the copied area has not been 'whitelisted' by setting useroffset and
>> usersize parameters for the kmem cache. As already discussed there
>> are two ways to mitigate this
> 
> I think we should just add a uverbs_copy_to() for integers, much like
> netlink does.
> 
> We already have various getters for integers..
> 
> Jason
> 

Perhaps. But I think the proposed fix is good in any case. It gets rid of a bunch of code that doesn't add anything.
The info leak police will come after kmalloc too. It is just a harder problem so your suggestion will still be a good one.

Re: Re another alternative to resolve hardened user copy warnings

[Jason Gunthorpe]

On Tue, Aug 25, 2020 at 12:12:18PM -0500, Bob Pearson wrote:
> On 8/25/20 11:53 AM, Jason Gunthorpe wrote:
> > On Tue, Aug 25, 2020 at 11:37:52AM -0500, Bob Pearson wrote:
> > 
> >> Currently only the rxe driver is exhibiting the issue of kernel
> >> warnings during qp create caused by recent kernel changes looking
> >> for potential information leaks to user space. The test which
> >> triggers this warning is very specific. It occurs when a portion of
> >> a kernel object stored in a slab cache is copied to user space and
> >> the copied area has not been 'whitelisted' by setting useroffset and
> >> usersize parameters for the kmem cache. As already discussed there
> >> are two ways to mitigate this
> > 
> > I think we should just add a uverbs_copy_to() for integers, much like
> > netlink does.
> > 
> > We already have various getters for integers..
> 
> Perhaps. But I think the proposed fix is good in any case. It gets
> rid of a bunch of code that doesn't add anything.  The info leak
> police will come after kmalloc too. It is just a harder problem so
> your suggestion will still be a good one.

Sure, if the caches are not so useful, then lets drop them.

Jason