Re: [PATCH 0/2] Add kvzalloc_struct to complement kvzalloc_array

2018-02-14 Thread Joe Perches
On Wed, 2018-02-14 at 11:56 -0800, Matthew Wilcox wrote:
> On Wed, Feb 14, 2018 at 11:43:46AM -0800, Joe Perches wrote:
> > On Wed, 2018-02-14 at 11:36 -0800, Matthew Wilcox wrote:
> > > If somebody wants them, then we can add them.
> > 
> > Yeah, but I don't think any of it is necessary.
> > 
> > How many of these struct+bufsize * count entries
> > actually exist?
> 
> Wrong question.  How many of them currently exist that don't check for
> integer overflow?  How many of them will be added in the future that
> will fail to check for integer overflow?
> 
> I chose five at random to fix as demonstration that the API is good.
> There are more; I imagine that Julia will be able to tell us how many.

No such conversions exist in the patch series
you submitted.

What are those?



Re: [PATCH 0/2] Add kvzalloc_struct to complement kvzalloc_array

2018-02-14 Thread Matthew Wilcox
On Wed, Feb 14, 2018 at 11:43:46AM -0800, Joe Perches wrote:
> On Wed, 2018-02-14 at 11:36 -0800, Matthew Wilcox wrote:
> > If somebody wants them, then we can add them.
> 
> Yeah, but I don't think any of it is necessary.
> 
> How many of these struct+bufsize * count entries
> actually exist?

Wrong question.  How many of them currently exist that don't check for
integer overflow?  How many of them will be added in the future that
will fail to check for integer overflow?

I chose five at random to fix as demonstration that the API is good.
There are more; I imagine that Julia will be able to tell us how many.


Re: [PATCH 0/2] Add kvzalloc_struct to complement kvzalloc_array

2018-02-14 Thread Joe Perches
On Wed, 2018-02-14 at 11:36 -0800, Matthew Wilcox wrote:
> On Wed, Feb 14, 2018 at 11:32:45AM -0800, Joe Perches wrote:
> > On Wed, 2018-02-14 at 11:23 -0800, Kees Cook wrote:
> > > On Wed, Feb 14, 2018 at 10:47 AM, Joe Perches  wrote:
> > > > I think expanding the number of allocation functions
> > > > is not necessary.
> > > 
> > > I think removing common mispatterns in favor of overflow-protected
> > > allocation functions makes sense.
> > 
> > Function symmetry matters too.
> > 
> > These allocation functions are specific to kvz
> > and are not symmetric for k, v, devm_
> > _node, and the like.
> 
> If somebody wants them, then we can add them.

Yeah, but I don't think any of it is necessary.

How many of these struct+bufsize * count entries
actually exist?



Re: [PATCH 0/2] Add kvzalloc_struct to complement kvzalloc_array

2018-02-14 Thread Matthew Wilcox
On Wed, Feb 14, 2018 at 11:32:45AM -0800, Joe Perches wrote:
> On Wed, 2018-02-14 at 11:23 -0800, Kees Cook wrote:
> > On Wed, Feb 14, 2018 at 10:47 AM, Joe Perches  wrote:
> > > I think expanding the number of allocation functions
> > > is not necessary.
> > 
> > I think removing common mispatterns in favor of overflow-protected
> > allocation functions makes sense.
> 
> Function symmetry matters too.
> 
> These allocation functions are specific to kvz
> and are not symmetric for k, v, devm_
> _node, and the like.

If somebody wants them, then we can add them.


Re: [PATCH 0/2] Add kvzalloc_struct to complement kvzalloc_array

2018-02-14 Thread Joe Perches
On Wed, 2018-02-14 at 11:23 -0800, Kees Cook wrote:
> On Wed, Feb 14, 2018 at 10:47 AM, Joe Perches  wrote:
> > On Wed, 2018-02-14 at 10:26 -0800, Matthew Wilcox wrote:
> > > From: Matthew Wilcox 
> > > 
> > > We all know the perils of multiplying a value provided from userspace
> > > by a constant and then allocating the resulting number of bytes.  That's
> > > why we have kvmalloc_array(), so we don't have to think about it.
> > > This solves the same problem when we embed one of these arrays in a
> > > struct like this:
> > > 
> > > struct {
> > >   int n;
> > >   unsigned long array[];
> > > };
> > 
> > I think expanding the number of allocation functions
> > is not necessary.
> 
> I think removing common mispatterns in favor of overflow-protected
> allocation functions makes sense.

Function symmetry matters too.

These allocation functions are specific to kvz
and are not symmetric for k, v, devm_
_node, and the like.



Re: [PATCH 0/2] Add kvzalloc_struct to complement kvzalloc_array

2018-02-14 Thread Kees Cook
On Wed, Feb 14, 2018 at 10:47 AM, Joe Perches  wrote:
> On Wed, 2018-02-14 at 10:26 -0800, Matthew Wilcox wrote:
>> From: Matthew Wilcox 
>>
>> We all know the perils of multiplying a value provided from userspace
>> by a constant and then allocating the resulting number of bytes.  That's
>> why we have kvmalloc_array(), so we don't have to think about it.
>> This solves the same problem when we embed one of these arrays in a
>> struct like this:
>>
>> struct {
>>   int n;
>>   unsigned long array[];
>> };
>
> I think expanding the number of allocation functions
> is not necessary.

I think removing common mispatterns in favor of overflow-protected
allocation functions makes sense.

-Kees

-- 
Kees Cook
Pixel Security


Re: [PATCH 0/2] Add kvzalloc_struct to complement kvzalloc_array

2018-02-14 Thread Joe Perches
On Wed, 2018-02-14 at 10:26 -0800, Matthew Wilcox wrote:
> From: Matthew Wilcox 
> 
> We all know the perils of multiplying a value provided from userspace
> by a constant and then allocating the resulting number of bytes.  That's
> why we have kvmalloc_array(), so we don't have to think about it.
> This solves the same problem when we embed one of these arrays in a
> struct like this:
> 
> struct {
>   int n;
>   unsigned long array[];
> };

I think expanding the number of allocation functions
is not necessary.