Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Ilya Dryomov]

On Wed, Jan 25, 2017 at 2:09 PM, Michal Hocko  wrote:
> On Wed 25-01-17 12:15:59, Vlastimil Babka wrote:
>> On 01/24/2017 04:00 PM, Michal Hocko wrote:
>> > > > Well, I am not opposed to kvmalloc_array but I would argue that this
>> > > > conversion cannot introduce new overflow issues. The code would have
>> > > > to be broken already because even though kmalloc_array checks for the
>> > > > overflow but vmalloc fallback doesn't...
>> > >
>> > > Yeah I agree, but if some of the places were really wrong, after the
>> > > conversion we won't see them anymore.
>> > >
>> > > > If there is a general interest for this API I can add it.
>> > >
>> > > I think it would be better, yes.
>> >
>> > OK, fair enough. I will fold the following into the original patch. I
>> > was little bit reluctant to create kvcalloc so I've made the original
>> > callers more talkative and added | __GFP_ZERO.
>>
>> Fair enough,
>>
>> > To be honest I do not
>> > really like how kcalloc...
>>
>> how kcalloc what?
>
> how kcalloc hides the GFP_ZERO and the name doesn't reflect that.

The userspace calloc() is specified to zero memory, so I'd say the name
does reflect that.

Thanks,

Ilya

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Ilya Dryomov]

On Wed, Jan 25, 2017 at 2:09 PM, Michal Hocko  wrote:
> On Wed 25-01-17 12:15:59, Vlastimil Babka wrote:
>> On 01/24/2017 04:00 PM, Michal Hocko wrote:
>> > > > Well, I am not opposed to kvmalloc_array but I would argue that this
>> > > > conversion cannot introduce new overflow issues. The code would have
>> > > > to be broken already because even though kmalloc_array checks for the
>> > > > overflow but vmalloc fallback doesn't...
>> > >
>> > > Yeah I agree, but if some of the places were really wrong, after the
>> > > conversion we won't see them anymore.
>> > >
>> > > > If there is a general interest for this API I can add it.
>> > >
>> > > I think it would be better, yes.
>> >
>> > OK, fair enough. I will fold the following into the original patch. I
>> > was little bit reluctant to create kvcalloc so I've made the original
>> > callers more talkative and added | __GFP_ZERO.
>>
>> Fair enough,
>>
>> > To be honest I do not
>> > really like how kcalloc...
>>
>> how kcalloc what?
>
> how kcalloc hides the GFP_ZERO and the name doesn't reflect that.

The userspace calloc() is specified to zero memory, so I'd say the name
does reflect that.

Thanks,

Ilya

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

On Wed 25-01-17 12:15:59, Vlastimil Babka wrote:
> On 01/24/2017 04:00 PM, Michal Hocko wrote:
> > > > Well, I am not opposed to kvmalloc_array but I would argue that this
> > > > conversion cannot introduce new overflow issues. The code would have
> > > > to be broken already because even though kmalloc_array checks for the
> > > > overflow but vmalloc fallback doesn't...
> > > 
> > > Yeah I agree, but if some of the places were really wrong, after the
> > > conversion we won't see them anymore.
> > > 
> > > > If there is a general interest for this API I can add it.
> > > 
> > > I think it would be better, yes.
> > 
> > OK, fair enough. I will fold the following into the original patch. I
> > was little bit reluctant to create kvcalloc so I've made the original
> > callers more talkative and added | __GFP_ZERO.
> 
> Fair enough,
> 
> > To be honest I do not
> > really like how kcalloc...
> 
> how kcalloc what?

how kcalloc hides the GFP_ZERO and the name doesn't reflect that.
 
> [...]
> > diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> > index cdc55d5ee4ad..eca16612b1ae 100644
> > --- a/net/netfilter/x_tables.c
> > +++ b/net/netfilter/x_tables.c
> > @@ -712,10 +712,7 @@ EXPORT_SYMBOL(xt_check_entry_offsets);
> >   */
> >  unsigned int *xt_alloc_entry_offsets(unsigned int size)
> >  {
> > -   if (size < (SIZE_MAX / sizeof(unsigned int)))
> > -   return kvzalloc(size * sizeof(unsigned int), GFP_KERNEL);
> > -
> > -   return NULL;
> > +   return kvmalloc_array(size * sizeof(unsigned int), GFP_KERNEL | 
> > __GFP_ZERO);
> 
> This one wouldn't compile.

fixed, thanks!

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

On Wed 25-01-17 12:15:59, Vlastimil Babka wrote:
> On 01/24/2017 04:00 PM, Michal Hocko wrote:
> > > > Well, I am not opposed to kvmalloc_array but I would argue that this
> > > > conversion cannot introduce new overflow issues. The code would have
> > > > to be broken already because even though kmalloc_array checks for the
> > > > overflow but vmalloc fallback doesn't...
> > > 
> > > Yeah I agree, but if some of the places were really wrong, after the
> > > conversion we won't see them anymore.
> > > 
> > > > If there is a general interest for this API I can add it.
> > > 
> > > I think it would be better, yes.
> > 
> > OK, fair enough. I will fold the following into the original patch. I
> > was little bit reluctant to create kvcalloc so I've made the original
> > callers more talkative and added | __GFP_ZERO.
> 
> Fair enough,
> 
> > To be honest I do not
> > really like how kcalloc...
> 
> how kcalloc what?

how kcalloc hides the GFP_ZERO and the name doesn't reflect that.
 
> [...]
> > diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> > index cdc55d5ee4ad..eca16612b1ae 100644
> > --- a/net/netfilter/x_tables.c
> > +++ b/net/netfilter/x_tables.c
> > @@ -712,10 +712,7 @@ EXPORT_SYMBOL(xt_check_entry_offsets);
> >   */
> >  unsigned int *xt_alloc_entry_offsets(unsigned int size)
> >  {
> > -   if (size < (SIZE_MAX / sizeof(unsigned int)))
> > -   return kvzalloc(size * sizeof(unsigned int), GFP_KERNEL);
> > -
> > -   return NULL;
> > +   return kvmalloc_array(size * sizeof(unsigned int), GFP_KERNEL | 
> > __GFP_ZERO);
> 
> This one wouldn't compile.

fixed, thanks!

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Vlastimil Babka]

On 01/24/2017 04:00 PM, Michal Hocko wrote:

> Well, I am not opposed to kvmalloc_array but I would argue that this
> conversion cannot introduce new overflow issues. The code would have
> to be broken already because even though kmalloc_array checks for the
> overflow but vmalloc fallback doesn't...

Yeah I agree, but if some of the places were really wrong, after the
conversion we won't see them anymore.

> If there is a general interest for this API I can add it.

I think it would be better, yes.


OK, fair enough. I will fold the following into the original patch. I
was little bit reluctant to create kvcalloc so I've made the original
callers more talkative and added | __GFP_ZERO.


Fair enough,


To be honest I do not
really like how kcalloc...


how kcalloc what?

[...]

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index cdc55d5ee4ad..eca16612b1ae 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -712,10 +712,7 @@ EXPORT_SYMBOL(xt_check_entry_offsets);
  */
 unsigned int *xt_alloc_entry_offsets(unsigned int size)
 {
-   if (size < (SIZE_MAX / sizeof(unsigned int)))
-   return kvzalloc(size * sizeof(unsigned int), GFP_KERNEL);
-
-   return NULL;
+   return kvmalloc_array(size * sizeof(unsigned int), GFP_KERNEL | 
__GFP_ZERO);


This one wouldn't compile.

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Vlastimil Babka]

On 01/24/2017 04:00 PM, Michal Hocko wrote:

> Well, I am not opposed to kvmalloc_array but I would argue that this
> conversion cannot introduce new overflow issues. The code would have
> to be broken already because even though kmalloc_array checks for the
> overflow but vmalloc fallback doesn't...

Yeah I agree, but if some of the places were really wrong, after the
conversion we won't see them anymore.

> If there is a general interest for this API I can add it.

I think it would be better, yes.


OK, fair enough. I will fold the following into the original patch. I
was little bit reluctant to create kvcalloc so I've made the original
callers more talkative and added | __GFP_ZERO.


Fair enough,


To be honest I do not
really like how kcalloc...


how kcalloc what?

[...]

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index cdc55d5ee4ad..eca16612b1ae 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -712,10 +712,7 @@ EXPORT_SYMBOL(xt_check_entry_offsets);
  */
 unsigned int *xt_alloc_entry_offsets(unsigned int size)
 {
-   if (size < (SIZE_MAX / sizeof(unsigned int)))
-   return kvzalloc(size * sizeof(unsigned int), GFP_KERNEL);
-
-   return NULL;
+   return kvmalloc_array(size * sizeof(unsigned int), GFP_KERNEL | 
__GFP_ZERO);


This one wouldn't compile.

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

On Fri 20-01-17 14:41:37, Vlastimil Babka wrote:
> On 01/12/2017 06:37 PM, Michal Hocko wrote:
> > On Thu 12-01-17 09:26:09, Kees Cook wrote:
> >> On Thu, Jan 12, 2017 at 7:37 AM, Michal Hocko  wrote:
> > [...]
> >>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> >>> index 4f74511015b8..e6bbb33d2956 100644
> >>> --- a/arch/s390/kvm/kvm-s390.c
> >>> +++ b/arch/s390/kvm/kvm-s390.c
> >>> @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, 
> >>> struct kvm_s390_skeys *args)
> >>> if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
> >>> return -EINVAL;
> >>>
> >>> -   keys = kmalloc_array(args->count, sizeof(uint8_t),
> >>> -GFP_KERNEL | __GFP_NOWARN);
> >>> -   if (!keys)
> >>> -   keys = vmalloc(sizeof(uint8_t) * args->count);
> >>> +   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
> >>
> >> Before doing this conversion, can we add a kvmalloc_array() API? This
> >> conversion could allow for the reintroduction of integer overflow
> >> flaws. (This particular situation isn't at risk since ->count is
> >> checked, but I'd prefer we not create a risky set of examples for
> >> using kvmalloc.)
> > 
> > Well, I am not opposed to kvmalloc_array but I would argue that this
> > conversion cannot introduce new overflow issues. The code would have
> > to be broken already because even though kmalloc_array checks for the
> > overflow but vmalloc fallback doesn't...
> 
> Yeah I agree, but if some of the places were really wrong, after the
> conversion we won't see them anymore.
> 
> > If there is a general interest for this API I can add it.
> 
> I think it would be better, yes.

OK, fair enough. I will fold the following into the original patch. I
was little bit reluctant to create kvcalloc so I've made the original
callers more talkative and added | __GFP_ZERO. To be honest I do not
really like how kcalloc...
---
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index e6bbb33d2956..aa558dce6bb4 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1126,7 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct 
kvm_s390_skeys *args)
if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
return -EINVAL;
 
-   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
+   keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
if (!keys)
return -ENOMEM;
 
@@ -1168,7 +1168,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct 
kvm_s390_skeys *args)
if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
return -EINVAL;
 
-   keys = kvmalloc(sizeof(uint8_t) * args->count, GFP_KERNEL);
+   keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
if (!keys)
return -ENOMEM;
 
diff --git a/drivers/net/ethernet/mellanox/mlx4/mr.c 
b/drivers/net/ethernet/mellanox/mlx4/mr.c
index 82354fd0a87e..6583d4601480 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mr.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mr.c
@@ -115,7 +115,7 @@ static int mlx4_buddy_init(struct mlx4_buddy *buddy, int 
max_order)
 
for (i = 0; i <= buddy->max_order; ++i) {
s = BITS_TO_LONGS(1 << (buddy->max_order - i));
-   buddy->bits[i] = kvzalloc(s * sizeof(long), GFP_KERNEL);
+   buddy->bits[i] = kvmalloc_array(s, sizeof(long), GFP_KERNEL | 
__GFP_ZERO);
if (!buddy->bits[i])
goto err_out_free;
}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 55fd570c3e1e..22c6e81d0c16 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -498,6 +498,14 @@ static inline void *kvzalloc(size_t size, gfp_t flags)
return kvmalloc(size, flags | __GFP_ZERO);
 }
 
+static inline void *kvmalloc_array(size_t n, size_t size, gfp_t flags)
+{
+   if (size != 0 && n > SIZE_MAX / size)
+   return NULL;
+
+   return kvmalloc(n * size, flags);
+}
+
 extern void kvfree(const void *addr);
 
 static inline atomic_t *compound_mapcount_ptr(struct page *page)
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 4ca30a951bbc..58ec07946fe6 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -320,7 +320,7 @@ static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
goto free_htab;
 
err = -ENOMEM;
-   htab->buckets = kvmalloc(htab->n_buckets * sizeof(struct bucket), 
GFP_USER);
+   htab->buckets = kvmalloc_array(htab->n_buckets, sizeof(struct bucket), 
GFP_USER);
if (!htab->buckets)
goto free_htab;
 
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 45c17b5562b5..8f9caf095172 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -957,7 +957,7 @@ EXPORT_SYMBOL(iov_iter_get_pages);
 
 static struct page **get_pages_array(size_t n)
 {
-   return kvmalloc(n * 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

On Fri 20-01-17 14:41:37, Vlastimil Babka wrote:
> On 01/12/2017 06:37 PM, Michal Hocko wrote:
> > On Thu 12-01-17 09:26:09, Kees Cook wrote:
> >> On Thu, Jan 12, 2017 at 7:37 AM, Michal Hocko  wrote:
> > [...]
> >>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> >>> index 4f74511015b8..e6bbb33d2956 100644
> >>> --- a/arch/s390/kvm/kvm-s390.c
> >>> +++ b/arch/s390/kvm/kvm-s390.c
> >>> @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, 
> >>> struct kvm_s390_skeys *args)
> >>> if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
> >>> return -EINVAL;
> >>>
> >>> -   keys = kmalloc_array(args->count, sizeof(uint8_t),
> >>> -GFP_KERNEL | __GFP_NOWARN);
> >>> -   if (!keys)
> >>> -   keys = vmalloc(sizeof(uint8_t) * args->count);
> >>> +   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
> >>
> >> Before doing this conversion, can we add a kvmalloc_array() API? This
> >> conversion could allow for the reintroduction of integer overflow
> >> flaws. (This particular situation isn't at risk since ->count is
> >> checked, but I'd prefer we not create a risky set of examples for
> >> using kvmalloc.)
> > 
> > Well, I am not opposed to kvmalloc_array but I would argue that this
> > conversion cannot introduce new overflow issues. The code would have
> > to be broken already because even though kmalloc_array checks for the
> > overflow but vmalloc fallback doesn't...
> 
> Yeah I agree, but if some of the places were really wrong, after the
> conversion we won't see them anymore.
> 
> > If there is a general interest for this API I can add it.
> 
> I think it would be better, yes.

OK, fair enough. I will fold the following into the original patch. I
was little bit reluctant to create kvcalloc so I've made the original
callers more talkative and added | __GFP_ZERO. To be honest I do not
really like how kcalloc...
---
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index e6bbb33d2956..aa558dce6bb4 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1126,7 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct 
kvm_s390_skeys *args)
if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
return -EINVAL;
 
-   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
+   keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
if (!keys)
return -ENOMEM;
 
@@ -1168,7 +1168,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct 
kvm_s390_skeys *args)
if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
return -EINVAL;
 
-   keys = kvmalloc(sizeof(uint8_t) * args->count, GFP_KERNEL);
+   keys = kvmalloc_array(args->count, sizeof(uint8_t), GFP_KERNEL);
if (!keys)
return -ENOMEM;
 
diff --git a/drivers/net/ethernet/mellanox/mlx4/mr.c 
b/drivers/net/ethernet/mellanox/mlx4/mr.c
index 82354fd0a87e..6583d4601480 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mr.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mr.c
@@ -115,7 +115,7 @@ static int mlx4_buddy_init(struct mlx4_buddy *buddy, int 
max_order)
 
for (i = 0; i <= buddy->max_order; ++i) {
s = BITS_TO_LONGS(1 << (buddy->max_order - i));
-   buddy->bits[i] = kvzalloc(s * sizeof(long), GFP_KERNEL);
+   buddy->bits[i] = kvmalloc_array(s, sizeof(long), GFP_KERNEL | 
__GFP_ZERO);
if (!buddy->bits[i])
goto err_out_free;
}
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 55fd570c3e1e..22c6e81d0c16 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -498,6 +498,14 @@ static inline void *kvzalloc(size_t size, gfp_t flags)
return kvmalloc(size, flags | __GFP_ZERO);
 }
 
+static inline void *kvmalloc_array(size_t n, size_t size, gfp_t flags)
+{
+   if (size != 0 && n > SIZE_MAX / size)
+   return NULL;
+
+   return kvmalloc(n * size, flags);
+}
+
 extern void kvfree(const void *addr);
 
 static inline atomic_t *compound_mapcount_ptr(struct page *page)
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 4ca30a951bbc..58ec07946fe6 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -320,7 +320,7 @@ static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
goto free_htab;
 
err = -ENOMEM;
-   htab->buckets = kvmalloc(htab->n_buckets * sizeof(struct bucket), 
GFP_USER);
+   htab->buckets = kvmalloc_array(htab->n_buckets, sizeof(struct bucket), 
GFP_USER);
if (!htab->buckets)
goto free_htab;
 
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 45c17b5562b5..8f9caf095172 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -957,7 +957,7 @@ EXPORT_SYMBOL(iov_iter_get_pages);
 
 static struct page **get_pages_array(size_t n)
 {
-   return kvmalloc(n * sizeof(struct page 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Vlastimil Babka]

On 01/12/2017 06:37 PM, Michal Hocko wrote:
> On Thu 12-01-17 09:26:09, Kees Cook wrote:
>> On Thu, Jan 12, 2017 at 7:37 AM, Michal Hocko  wrote:
> [...]
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index 4f74511015b8..e6bbb33d2956 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, 
>>> struct kvm_s390_skeys *args)
>>> if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
>>> return -EINVAL;
>>>
>>> -   keys = kmalloc_array(args->count, sizeof(uint8_t),
>>> -GFP_KERNEL | __GFP_NOWARN);
>>> -   if (!keys)
>>> -   keys = vmalloc(sizeof(uint8_t) * args->count);
>>> +   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
>>
>> Before doing this conversion, can we add a kvmalloc_array() API? This
>> conversion could allow for the reintroduction of integer overflow
>> flaws. (This particular situation isn't at risk since ->count is
>> checked, but I'd prefer we not create a risky set of examples for
>> using kvmalloc.)
> 
> Well, I am not opposed to kvmalloc_array but I would argue that this
> conversion cannot introduce new overflow issues. The code would have
> to be broken already because even though kmalloc_array checks for the
> overflow but vmalloc fallback doesn't...

Yeah I agree, but if some of the places were really wrong, after the
conversion we won't see them anymore.

> If there is a general interest for this API I can add it.

I think it would be better, yes.

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Vlastimil Babka]

On 01/12/2017 06:37 PM, Michal Hocko wrote:
> On Thu 12-01-17 09:26:09, Kees Cook wrote:
>> On Thu, Jan 12, 2017 at 7:37 AM, Michal Hocko  wrote:
> [...]
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index 4f74511015b8..e6bbb33d2956 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, 
>>> struct kvm_s390_skeys *args)
>>> if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
>>> return -EINVAL;
>>>
>>> -   keys = kmalloc_array(args->count, sizeof(uint8_t),
>>> -GFP_KERNEL | __GFP_NOWARN);
>>> -   if (!keys)
>>> -   keys = vmalloc(sizeof(uint8_t) * args->count);
>>> +   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
>>
>> Before doing this conversion, can we add a kvmalloc_array() API? This
>> conversion could allow for the reintroduction of integer overflow
>> flaws. (This particular situation isn't at risk since ->count is
>> checked, but I'd prefer we not create a risky set of examples for
>> using kvmalloc.)
> 
> Well, I am not opposed to kvmalloc_array but I would argue that this
> conversion cannot introduce new overflow issues. The code would have
> to be broken already because even though kmalloc_array checks for the
> overflow but vmalloc fallback doesn't...

Yeah I agree, but if some of the places were really wrong, after the
conversion we won't see them anymore.

> If there is a general interest for this API I can add it.

I think it would be better, yes.

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Leon Romanovsky]

On Mon, Jan 16, 2017 at 08:33:11AM +0100, Michal Hocko wrote:
> On Sat 14-01-17 12:56:32, Leon Romanovsky wrote:
> [...]
> > Hi Michal,
> >
> > I don't see mlx5_vzalloc in the changed list. Any reason why did you skip 
> > it?
> >
> >  881 static inline void *mlx5_vzalloc(unsigned long size)
> >  882 {
> >  883 void *rtn;
> >  884
> >  885 rtn = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
> >  886 if (!rtn)
> >  887 rtn = vzalloc(size);
> >  888 return rtn;
> >  889 }
>
> No reason to skip it, I just didn't see it. I will fold the following in
> if you are OK with it

Sure, no problem.
Once, the patch set is accepted, we (Mellanox) will get rid of mlx5_vzalloc().

Thanks


> ---
> diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
> index cdd2bd62f86d..5e6063170e48 100644
> --- a/include/linux/mlx5/driver.h
> +++ b/include/linux/mlx5/driver.h
> @@ -874,12 +874,7 @@ static inline u16 cmdif_rev(struct mlx5_core_dev *dev)
>
>  static inline void *mlx5_vzalloc(unsigned long size)
>  {
> - void *rtn;
> -
> - rtn = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
> - if (!rtn)
> - rtn = vzalloc(size);
> - return rtn;
> + return kvzalloc(GFP_KERNEL, size);
>  }
>
>  static inline u32 mlx5_base_mkey(const u32 key)
>
> --
> Michal Hocko
> SUSE Labs
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 


signature.asc
Description: PGP signature

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Leon Romanovsky]

On Mon, Jan 16, 2017 at 08:33:11AM +0100, Michal Hocko wrote:
> On Sat 14-01-17 12:56:32, Leon Romanovsky wrote:
> [...]
> > Hi Michal,
> >
> > I don't see mlx5_vzalloc in the changed list. Any reason why did you skip 
> > it?
> >
> >  881 static inline void *mlx5_vzalloc(unsigned long size)
> >  882 {
> >  883 void *rtn;
> >  884
> >  885 rtn = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
> >  886 if (!rtn)
> >  887 rtn = vzalloc(size);
> >  888 return rtn;
> >  889 }
>
> No reason to skip it, I just didn't see it. I will fold the following in
> if you are OK with it

Sure, no problem.
Once, the patch set is accepted, we (Mellanox) will get rid of mlx5_vzalloc().

Thanks


> ---
> diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
> index cdd2bd62f86d..5e6063170e48 100644
> --- a/include/linux/mlx5/driver.h
> +++ b/include/linux/mlx5/driver.h
> @@ -874,12 +874,7 @@ static inline u16 cmdif_rev(struct mlx5_core_dev *dev)
>
>  static inline void *mlx5_vzalloc(unsigned long size)
>  {
> - void *rtn;
> -
> - rtn = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
> - if (!rtn)
> - rtn = vzalloc(size);
> - return rtn;
> + return kvzalloc(GFP_KERNEL, size);
>  }
>
>  static inline u32 mlx5_base_mkey(const u32 key)
>
> --
> Michal Hocko
> SUSE Labs
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 


signature.asc
Description: PGP signature

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Tariq Toukan]

On 12/01/2017 5:37 PM, Michal Hocko wrote:

From: Michal Hocko 

There are many code paths opencoding kvmalloc. Let's use the helper
instead. The main difference to kvmalloc is that those users are usually
not considering all the aspects of the memory allocator. E.g. allocation
requests < 64kB are basically never failing and invoke OOM killer to
satisfy the allocation. This sounds too disruptive for something that
has a reasonable fallback - the vmalloc. On the other hand those
requests might fallback to vmalloc even when the memory allocator would
succeed after several more reclaim/compaction attempts previously. There
is no guarantee something like that happens though.

This patch converts many of those places to kv[mz]alloc* helpers because
they are more conservative.

Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
Cc: Herbert Xu 
Cc: Anton Vorontsov 
Cc: Colin Cross 
Cc: Kees Cook 
Cc: Tony Luck 
Cc: "Rafael J. Wysocki" 
Cc: Ben Skeggs 
Cc: Kent Overstreet 
Cc: Santosh Raspatur 
Cc: Hariprasad S 
Cc: Tariq Toukan 
Cc: Yishai Hadas 
Cc: Dan Williams 
Cc: Oleg Drokin 
Cc: Andreas Dilger 
Cc: Boris Ostrovsky 
Cc: David Sterba 
Cc: "Yan, Zheng" 
Cc: Ilya Dryomov 
Cc: Alexander Viro 
Cc: Alexei Starovoitov 
Cc: Eric Dumazet 
Cc: net...@vger.kernel.org
Signed-off-by: Michal Hocko 
---

Acked-by: Tariq Toukan 
For the mlx4 parts.

Regards.
Tariq

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Tariq Toukan]

On 12/01/2017 5:37 PM, Michal Hocko wrote:

From: Michal Hocko 

There are many code paths opencoding kvmalloc. Let's use the helper
instead. The main difference to kvmalloc is that those users are usually
not considering all the aspects of the memory allocator. E.g. allocation
requests < 64kB are basically never failing and invoke OOM killer to
satisfy the allocation. This sounds too disruptive for something that
has a reasonable fallback - the vmalloc. On the other hand those
requests might fallback to vmalloc even when the memory allocator would
succeed after several more reclaim/compaction attempts previously. There
is no guarantee something like that happens though.

This patch converts many of those places to kv[mz]alloc* helpers because
they are more conservative.

Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
Cc: Herbert Xu 
Cc: Anton Vorontsov 
Cc: Colin Cross 
Cc: Kees Cook 
Cc: Tony Luck 
Cc: "Rafael J. Wysocki" 
Cc: Ben Skeggs 
Cc: Kent Overstreet 
Cc: Santosh Raspatur 
Cc: Hariprasad S 
Cc: Tariq Toukan 
Cc: Yishai Hadas 
Cc: Dan Williams 
Cc: Oleg Drokin 
Cc: Andreas Dilger 
Cc: Boris Ostrovsky 
Cc: David Sterba 
Cc: "Yan, Zheng" 
Cc: Ilya Dryomov 
Cc: Alexander Viro 
Cc: Alexei Starovoitov 
Cc: Eric Dumazet 
Cc: net...@vger.kernel.org
Signed-off-by: Michal Hocko 
---

Acked-by: Tariq Toukan 
For the mlx4 parts.

Regards.
Tariq

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

On Sat 14-01-17 12:56:32, Leon Romanovsky wrote:
[...]
> Hi Michal,
> 
> I don't see mlx5_vzalloc in the changed list. Any reason why did you skip it?
> 
>  881 static inline void *mlx5_vzalloc(unsigned long size)
>  882 {
>  883 void *rtn;
>  884
>  885 rtn = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
>  886 if (!rtn)
>  887 rtn = vzalloc(size);
>  888 return rtn;
>  889 }

No reason to skip it, I just didn't see it. I will fold the following in
if you are OK with it
---
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index cdd2bd62f86d..5e6063170e48 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -874,12 +874,7 @@ static inline u16 cmdif_rev(struct mlx5_core_dev *dev)
 
 static inline void *mlx5_vzalloc(unsigned long size)
 {
-   void *rtn;
-
-   rtn = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
-   if (!rtn)
-   rtn = vzalloc(size);
-   return rtn;
+   return kvzalloc(GFP_KERNEL, size);
 }
 
 static inline u32 mlx5_base_mkey(const u32 key)

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

On Sat 14-01-17 12:56:32, Leon Romanovsky wrote:
[...]
> Hi Michal,
> 
> I don't see mlx5_vzalloc in the changed list. Any reason why did you skip it?
> 
>  881 static inline void *mlx5_vzalloc(unsigned long size)
>  882 {
>  883 void *rtn;
>  884
>  885 rtn = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
>  886 if (!rtn)
>  887 rtn = vzalloc(size);
>  888 return rtn;
>  889 }

No reason to skip it, I just didn't see it. I will fold the following in
if you are OK with it
---
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index cdd2bd62f86d..5e6063170e48 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -874,12 +874,7 @@ static inline u16 cmdif_rev(struct mlx5_core_dev *dev)
 
 static inline void *mlx5_vzalloc(unsigned long size)
 {
-   void *rtn;
-
-   rtn = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
-   if (!rtn)
-   rtn = vzalloc(size);
-   return rtn;
+   return kvzalloc(GFP_KERNEL, size);
 }
 
 static inline u32 mlx5_base_mkey(const u32 key)

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Leon Romanovsky]

On Thu, Jan 12, 2017 at 04:37:16PM +0100, Michal Hocko wrote:
> From: Michal Hocko 
>
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
>
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.
>
> Cc: Martin Schwidefsky 
> Cc: Heiko Carstens 
> Cc: Herbert Xu 
> Cc: Anton Vorontsov 
> Cc: Colin Cross 
> Cc: Kees Cook 
> Cc: Tony Luck 
> Cc: "Rafael J. Wysocki" 
> Cc: Ben Skeggs 
> Cc: Kent Overstreet 
> Cc: Santosh Raspatur 
> Cc: Hariprasad S 
> Cc: Tariq Toukan 
> Cc: Yishai Hadas 
> Cc: Dan Williams 
> Cc: Oleg Drokin 
> Cc: Andreas Dilger 
> Cc: Boris Ostrovsky 
> Cc: David Sterba 
> Cc: "Yan, Zheng" 
> Cc: Ilya Dryomov 
> Cc: Alexander Viro 
> Cc: Alexei Starovoitov 
> Cc: Eric Dumazet 
> Cc: net...@vger.kernel.org
> Signed-off-by: Michal Hocko 
> ---
>  arch/s390/kvm/kvm-s390.c   | 10 ++-
>  crypto/lzo.c   |  4 +--
>  drivers/acpi/apei/erst.c   |  8 ++---
>  drivers/char/agp/generic.c |  8 +
>  drivers/gpu/drm/nouveau/nouveau_gem.c  |  4 +--
>  drivers/md/bcache/util.h   | 12 ++--
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h|  3 --
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--
>  drivers/net/ethernet/chelsio/cxgb3/l2t.c   |  2 +-
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 31 
>  drivers/net/ethernet/mellanox/mlx4/en_tx.c |  9 ++
>  drivers/net/ethernet/mellanox/mlx4/mr.c|  9 ++
>  drivers/nvdimm/dimm_devs.c |  5 +---
>  .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +--
>  drivers/xen/evtchn.c   | 14 +
>  fs/btrfs/ctree.c   |  9 ++
>  fs/btrfs/ioctl.c   |  9 ++
>  fs/btrfs/send.c| 27 ++---
>  fs/ceph/file.c |  9 ++
>  fs/select.c|  5 +---
>  fs/xattr.c | 27 ++---
>  kernel/bpf/hashtab.c   | 11 ++-
>  lib/iov_iter.c |  5 +---
>  mm/frame_vector.c  |  5 +---
>  net/ipv4/inet_hashtables.c |  6 +---
>  net/ipv4/tcp_metrics.c |  5 +---
>  net/mpls/af_mpls.c |  5 +---
>  net/netfilter/x_tables.c   | 34 
> ++
>  net/netfilter/xt_recent.c  |  5 +---
>  net/sched/sch_choke.c  |  5 +---
>  net/sched/sch_fq_codel.c   | 26 -
>  net/sched/sch_hhf.c| 33 ++---
>  net/sched/sch_netem.c  |  6 +---
>  net/sched/sch_sfq.c|  6 +---
>  security/keys/keyctl.c | 22 --
>  35 files changed, 96 insertions(+), 319 deletions(-)

Hi Michal,

I don't see mlx5_vzalloc in the changed list. Any reason why did you skip it?

 881 static inline void *mlx5_vzalloc(unsigned long size)
 882 {
 883 void *rtn;
 884
 885 rtn = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
 886 if (!rtn)
 887 rtn = vzalloc(size);
 888 return rtn;
 889 }

Thanks


signature.asc
Description: PGP signature

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Leon Romanovsky]

On Thu, Jan 12, 2017 at 04:37:16PM +0100, Michal Hocko wrote:
> From: Michal Hocko 
>
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
>
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.
>
> Cc: Martin Schwidefsky 
> Cc: Heiko Carstens 
> Cc: Herbert Xu 
> Cc: Anton Vorontsov 
> Cc: Colin Cross 
> Cc: Kees Cook 
> Cc: Tony Luck 
> Cc: "Rafael J. Wysocki" 
> Cc: Ben Skeggs 
> Cc: Kent Overstreet 
> Cc: Santosh Raspatur 
> Cc: Hariprasad S 
> Cc: Tariq Toukan 
> Cc: Yishai Hadas 
> Cc: Dan Williams 
> Cc: Oleg Drokin 
> Cc: Andreas Dilger 
> Cc: Boris Ostrovsky 
> Cc: David Sterba 
> Cc: "Yan, Zheng" 
> Cc: Ilya Dryomov 
> Cc: Alexander Viro 
> Cc: Alexei Starovoitov 
> Cc: Eric Dumazet 
> Cc: net...@vger.kernel.org
> Signed-off-by: Michal Hocko 
> ---
>  arch/s390/kvm/kvm-s390.c   | 10 ++-
>  crypto/lzo.c   |  4 +--
>  drivers/acpi/apei/erst.c   |  8 ++---
>  drivers/char/agp/generic.c |  8 +
>  drivers/gpu/drm/nouveau/nouveau_gem.c  |  4 +--
>  drivers/md/bcache/util.h   | 12 ++--
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h|  3 --
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--
>  drivers/net/ethernet/chelsio/cxgb3/l2t.c   |  2 +-
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 31 
>  drivers/net/ethernet/mellanox/mlx4/en_tx.c |  9 ++
>  drivers/net/ethernet/mellanox/mlx4/mr.c|  9 ++
>  drivers/nvdimm/dimm_devs.c |  5 +---
>  .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +--
>  drivers/xen/evtchn.c   | 14 +
>  fs/btrfs/ctree.c   |  9 ++
>  fs/btrfs/ioctl.c   |  9 ++
>  fs/btrfs/send.c| 27 ++---
>  fs/ceph/file.c |  9 ++
>  fs/select.c|  5 +---
>  fs/xattr.c | 27 ++---
>  kernel/bpf/hashtab.c   | 11 ++-
>  lib/iov_iter.c |  5 +---
>  mm/frame_vector.c  |  5 +---
>  net/ipv4/inet_hashtables.c |  6 +---
>  net/ipv4/tcp_metrics.c |  5 +---
>  net/mpls/af_mpls.c |  5 +---
>  net/netfilter/x_tables.c   | 34 
> ++
>  net/netfilter/xt_recent.c  |  5 +---
>  net/sched/sch_choke.c  |  5 +---
>  net/sched/sch_fq_codel.c   | 26 -
>  net/sched/sch_hhf.c| 33 ++---
>  net/sched/sch_netem.c  |  6 +---
>  net/sched/sch_sfq.c|  6 +---
>  security/keys/keyctl.c | 22 --
>  35 files changed, 96 insertions(+), 319 deletions(-)

Hi Michal,

I don't see mlx5_vzalloc in the changed list. Any reason why did you skip it?

 881 static inline void *mlx5_vzalloc(unsigned long size)
 882 {
 883 void *rtn;
 884
 885 rtn = kzalloc(size, GFP_KERNEL | __GFP_NOWARN);
 886 if (!rtn)
 887 rtn = vzalloc(size);
 888 return rtn;
 889 }

Thanks


signature.asc
Description: PGP signature

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

On Sat 14-01-17 12:01:50, Tetsuo Handa wrote:
> On 2017/01/13 2:29, Michal Hocko wrote:
> > Ilya has noticed that I've screwed up some k[zc]alloc conversions and
> > didn't use the kvzalloc. This is an updated patch with some acks
> > collected on the way
> > ---
> > From a7b89c6d0a3c685045e37740c8f97b065f37e0a4 Mon Sep 17 00:00:00 2001
> > From: Michal Hocko 
> > Date: Wed, 4 Jan 2017 13:30:32 +0100
> > Subject: [PATCH] treewide: use kv[mz]alloc* rather than opencoded variants
> > 
> > There are many code paths opencoding kvmalloc. Let's use the helper
> > instead. The main difference to kvmalloc is that those users are usually
> > not considering all the aspects of the memory allocator. E.g. allocation
> > requests < 64kB are basically never failing and invoke OOM killer to
> 
> Isn't this "requests <= 32kB" because allocation requests for 33kB will be
> rounded up to 64kB?

Yes

> Same for "smaller than 64kB" in PATCH 6/6. But strictly speaking, isn't
> it bogus to refer actual size because PAGE_SIZE is not always 4096?

This is just an example and I didn't want to pull
PAGE_ALLOC_COSTLY_ORDER here. So I've instead fixed the wording to:
"
E.g. allocation requests <= 32kB (with 4kB pages) are basically never
failing and invoke OOM killer to satisfy the allocation.
"
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

On Sat 14-01-17 12:01:50, Tetsuo Handa wrote:
> On 2017/01/13 2:29, Michal Hocko wrote:
> > Ilya has noticed that I've screwed up some k[zc]alloc conversions and
> > didn't use the kvzalloc. This is an updated patch with some acks
> > collected on the way
> > ---
> > From a7b89c6d0a3c685045e37740c8f97b065f37e0a4 Mon Sep 17 00:00:00 2001
> > From: Michal Hocko 
> > Date: Wed, 4 Jan 2017 13:30:32 +0100
> > Subject: [PATCH] treewide: use kv[mz]alloc* rather than opencoded variants
> > 
> > There are many code paths opencoding kvmalloc. Let's use the helper
> > instead. The main difference to kvmalloc is that those users are usually
> > not considering all the aspects of the memory allocator. E.g. allocation
> > requests < 64kB are basically never failing and invoke OOM killer to
> 
> Isn't this "requests <= 32kB" because allocation requests for 33kB will be
> rounded up to 64kB?

Yes

> Same for "smaller than 64kB" in PATCH 6/6. But strictly speaking, isn't
> it bogus to refer actual size because PAGE_SIZE is not always 4096?

This is just an example and I didn't want to pull
PAGE_ALLOC_COSTLY_ORDER here. So I've instead fixed the wording to:
"
E.g. allocation requests <= 32kB (with 4kB pages) are basically never
failing and invoke OOM killer to satisfy the allocation.
"
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Tetsuo Handa]

On 2017/01/13 2:29, Michal Hocko wrote:
> Ilya has noticed that I've screwed up some k[zc]alloc conversions and
> didn't use the kvzalloc. This is an updated patch with some acks
> collected on the way
> ---
> From a7b89c6d0a3c685045e37740c8f97b065f37e0a4 Mon Sep 17 00:00:00 2001
> From: Michal Hocko 
> Date: Wed, 4 Jan 2017 13:30:32 +0100
> Subject: [PATCH] treewide: use kv[mz]alloc* rather than opencoded variants
> 
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to

Isn't this "requests <= 32kB" because allocation requests for 33kB will be
rounded up to 64kB?

Same for "smaller than 64kB" in PATCH 6/6. But strictly speaking, isn't
it bogus to refer actual size because PAGE_SIZE is not always 4096?

-- arch/ia64/include/asm/page.h --
/*
 * PAGE_SHIFT determines the actual kernel page size.
 */
#if defined(CONFIG_IA64_PAGE_SIZE_4KB)
# define PAGE_SHIFT 12
#elif defined(CONFIG_IA64_PAGE_SIZE_8KB)
# define PAGE_SHIFT 13
#elif defined(CONFIG_IA64_PAGE_SIZE_16KB)
# define PAGE_SHIFT 14
#elif defined(CONFIG_IA64_PAGE_SIZE_64KB)
# define PAGE_SHIFT 16
#else
# error Unsupported page size!
#endif

#define PAGE_SIZE   (__IA64_UL_CONST(1) << PAGE_SHIFT)


> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
> 
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Tetsuo Handa]

On 2017/01/13 2:29, Michal Hocko wrote:
> Ilya has noticed that I've screwed up some k[zc]alloc conversions and
> didn't use the kvzalloc. This is an updated patch with some acks
> collected on the way
> ---
> From a7b89c6d0a3c685045e37740c8f97b065f37e0a4 Mon Sep 17 00:00:00 2001
> From: Michal Hocko 
> Date: Wed, 4 Jan 2017 13:30:32 +0100
> Subject: [PATCH] treewide: use kv[mz]alloc* rather than opencoded variants
> 
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to

Isn't this "requests <= 32kB" because allocation requests for 33kB will be
rounded up to 64kB?

Same for "smaller than 64kB" in PATCH 6/6. But strictly speaking, isn't
it bogus to refer actual size because PAGE_SIZE is not always 4096?

-- arch/ia64/include/asm/page.h --
/*
 * PAGE_SHIFT determines the actual kernel page size.
 */
#if defined(CONFIG_IA64_PAGE_SIZE_4KB)
# define PAGE_SHIFT 12
#elif defined(CONFIG_IA64_PAGE_SIZE_8KB)
# define PAGE_SHIFT 13
#elif defined(CONFIG_IA64_PAGE_SIZE_16KB)
# define PAGE_SHIFT 14
#elif defined(CONFIG_IA64_PAGE_SIZE_64KB)
# define PAGE_SHIFT 16
#else
# error Unsupported page size!
#endif

#define PAGE_SIZE   (__IA64_UL_CONST(1) << PAGE_SHIFT)


> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
> 
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Dilger, Andreas]

> On Jan 12, 2017, at 08:37, Michal Hocko  wrote:
> 
> From: Michal Hocko 
> 
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
> 
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.
> 
> Signed-off-by: Michal Hocko 

Lustre part can be
Acked-by: Andreas Dilger 

[snip]

> diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-mem.c 
> b/drivers/staging/lustre/lnet/libcfs/linux/linux-mem.c
> index a6a76a681ea9..8f638267e704 100644
> --- a/drivers/staging/lustre/lnet/libcfs/linux/linux-mem.c
> +++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-mem.c
> @@ -45,15 +45,6 @@ EXPORT_SYMBOL(libcfs_kvzalloc);
> void *libcfs_kvzalloc_cpt(struct cfs_cpt_table *cptab, int cpt, size_t size,
> gfp_t flags)
> {
> - void *ret;
> -
> - ret = kzalloc_node(size, flags | __GFP_NOWARN,
> -cfs_cpt_spread_node(cptab, cpt));
> - if (!ret) {
> - WARN_ON(!(flags & (__GFP_FS | __GFP_HIGH)));
> - ret = vmalloc_node(size, cfs_cpt_spread_node(cptab, cpt));
> - }
> -
> - return ret;
> + return kvzalloc_node(size, flags, cfs_cpt_spread_node(cptab, cpt));
> }
> EXPORT_SYMBOL(libcfs_kvzalloc_cpt);

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Dilger, Andreas]

> On Jan 12, 2017, at 08:37, Michal Hocko  wrote:
> 
> From: Michal Hocko 
> 
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
> 
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.
> 
> Signed-off-by: Michal Hocko 

Lustre part can be
Acked-by: Andreas Dilger 

[snip]

> diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-mem.c 
> b/drivers/staging/lustre/lnet/libcfs/linux/linux-mem.c
> index a6a76a681ea9..8f638267e704 100644
> --- a/drivers/staging/lustre/lnet/libcfs/linux/linux-mem.c
> +++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-mem.c
> @@ -45,15 +45,6 @@ EXPORT_SYMBOL(libcfs_kvzalloc);
> void *libcfs_kvzalloc_cpt(struct cfs_cpt_table *cptab, int cpt, size_t size,
> gfp_t flags)
> {
> - void *ret;
> -
> - ret = kzalloc_node(size, flags | __GFP_NOWARN,
> -cfs_cpt_spread_node(cptab, cpt));
> - if (!ret) {
> - WARN_ON(!(flags & (__GFP_FS | __GFP_HIGH)));
> - ret = vmalloc_node(size, cfs_cpt_spread_node(cptab, cpt));
> - }
> -
> - return ret;
> + return kvzalloc_node(size, flags, cfs_cpt_spread_node(cptab, cpt));
> }
> EXPORT_SYMBOL(libcfs_kvzalloc_cpt);

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Boris Ostrovsky]

> diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
> index 6890897a6f30..10f1ef582659 100644
> --- a/drivers/xen/evtchn.c
> +++ b/drivers/xen/evtchn.c
> @@ -87,18 +87,6 @@ struct user_evtchn {
>   bool enabled;
>  };
>  
> -static evtchn_port_t *evtchn_alloc_ring(unsigned int size)
> -{
> - evtchn_port_t *ring;
> - size_t s = size * sizeof(*ring);
> -
> - ring = kmalloc(s, GFP_KERNEL);
> - if (!ring)
> - ring = vmalloc(s);
> -
> - return ring;
> -}
> -
>  static void evtchn_free_ring(evtchn_port_t *ring)
>  {
>   kvfree(ring);
> @@ -334,7 +322,7 @@ static int evtchn_resize_ring(struct per_user_data *u)
>   else
>   new_size = 2 * u->ring_size;
>  
> - new_ring = evtchn_alloc_ring(new_size);
> + new_ring = kvmalloc(new_size * sizeof(*new_ring), GFP_KERNEL);
>   if (!new_ring)
>   return -ENOMEM;
>  

Xen bits:

Reviewed-by: Boris Ostrovsky 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Boris Ostrovsky]

> diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
> index 6890897a6f30..10f1ef582659 100644
> --- a/drivers/xen/evtchn.c
> +++ b/drivers/xen/evtchn.c
> @@ -87,18 +87,6 @@ struct user_evtchn {
>   bool enabled;
>  };
>  
> -static evtchn_port_t *evtchn_alloc_ring(unsigned int size)
> -{
> - evtchn_port_t *ring;
> - size_t s = size * sizeof(*ring);
> -
> - ring = kmalloc(s, GFP_KERNEL);
> - if (!ring)
> - ring = vmalloc(s);
> -
> - return ring;
> -}
> -
>  static void evtchn_free_ring(evtchn_port_t *ring)
>  {
>   kvfree(ring);
> @@ -334,7 +322,7 @@ static int evtchn_resize_ring(struct per_user_data *u)
>   else
>   new_size = 2 * u->ring_size;
>  
> - new_ring = evtchn_alloc_ring(new_size);
> + new_ring = kvmalloc(new_size * sizeof(*new_ring), GFP_KERNEL);
>   if (!new_ring)
>   return -ENOMEM;
>  

Xen bits:

Reviewed-by: Boris Ostrovsky 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

On Thu 12-01-17 09:26:09, Kees Cook wrote:
> On Thu, Jan 12, 2017 at 7:37 AM, Michal Hocko  wrote:
[...]
> > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> > index 4f74511015b8..e6bbb33d2956 100644
> > --- a/arch/s390/kvm/kvm-s390.c
> > +++ b/arch/s390/kvm/kvm-s390.c
> > @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, 
> > struct kvm_s390_skeys *args)
> > if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
> > return -EINVAL;
> >
> > -   keys = kmalloc_array(args->count, sizeof(uint8_t),
> > -GFP_KERNEL | __GFP_NOWARN);
> > -   if (!keys)
> > -   keys = vmalloc(sizeof(uint8_t) * args->count);
> > +   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
> 
> Before doing this conversion, can we add a kvmalloc_array() API? This
> conversion could allow for the reintroduction of integer overflow
> flaws. (This particular situation isn't at risk since ->count is
> checked, but I'd prefer we not create a risky set of examples for
> using kvmalloc.)

Well, I am not opposed to kvmalloc_array but I would argue that this
conversion cannot introduce new overflow issues. The code would have
to be broken already because even though kmalloc_array checks for the
overflow but vmalloc fallback doesn't...

If there is a general interest for this API I can add it.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

On Thu 12-01-17 09:26:09, Kees Cook wrote:
> On Thu, Jan 12, 2017 at 7:37 AM, Michal Hocko  wrote:
[...]
> > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> > index 4f74511015b8..e6bbb33d2956 100644
> > --- a/arch/s390/kvm/kvm-s390.c
> > +++ b/arch/s390/kvm/kvm-s390.c
> > @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, 
> > struct kvm_s390_skeys *args)
> > if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
> > return -EINVAL;
> >
> > -   keys = kmalloc_array(args->count, sizeof(uint8_t),
> > -GFP_KERNEL | __GFP_NOWARN);
> > -   if (!keys)
> > -   keys = vmalloc(sizeof(uint8_t) * args->count);
> > +   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
> 
> Before doing this conversion, can we add a kvmalloc_array() API? This
> conversion could allow for the reintroduction of integer overflow
> flaws. (This particular situation isn't at risk since ->count is
> checked, but I'd prefer we not create a risky set of examples for
> using kvmalloc.)

Well, I am not opposed to kvmalloc_array but I would argue that this
conversion cannot introduce new overflow issues. The code would have
to be broken already because even though kmalloc_array checks for the
overflow but vmalloc fallback doesn't...

If there is a general interest for this API I can add it.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

Ilya has noticed that I've screwed up some k[zc]alloc conversions and
didn't use the kvzalloc. This is an updated patch with some acks
collected on the way
---
>From a7b89c6d0a3c685045e37740c8f97b065f37e0a4 Mon Sep 17 00:00:00 2001
From: Michal Hocko 
Date: Wed, 4 Jan 2017 13:30:32 +0100
Subject: [PATCH] treewide: use kv[mz]alloc* rather than opencoded variants

There are many code paths opencoding kvmalloc. Let's use the helper
instead. The main difference to kvmalloc is that those users are usually
not considering all the aspects of the memory allocator. E.g. allocation
requests < 64kB are basically never failing and invoke OOM killer to
satisfy the allocation. This sounds too disruptive for something that
has a reasonable fallback - the vmalloc. On the other hand those
requests might fallback to vmalloc even when the memory allocator would
succeed after several more reclaim/compaction attempts previously. There
is no guarantee something like that happens though.

This patch converts many of those places to kv[mz]alloc* helpers because
they are more conservative.

Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
Cc: Herbert Xu 
Cc: Anton Vorontsov 
Cc: Colin Cross 
Cc: Kees Cook 
Cc: Tony Luck 
Cc: "Rafael J. Wysocki" 
Cc: Ben Skeggs 
Cc: Kent Overstreet 
Cc: Santosh Raspatur 
Cc: Hariprasad S 
Cc: Tariq Toukan 
Cc: Yishai Hadas 
Cc: Oleg Drokin 
Cc: Andreas Dilger 
Cc: Boris Ostrovsky 
Cc: "Yan, Zheng" 
Cc: Alexander Viro 
Cc: Alexei Starovoitov 
Cc: Eric Dumazet 
Cc: net...@vger.kernel.org
Acked-by: Christian Borntraeger  # KVM/s390
Acked-by: Dan Williams  # nvdim
Acked-by: David Sterba  # btrfs
Acked-by: Ilya Dryomov  # Ceph
Signed-off-by: Michal Hocko 
---
 arch/s390/kvm/kvm-s390.c   | 10 ++-
 crypto/lzo.c   |  4 +--
 drivers/acpi/apei/erst.c   |  8 ++---
 drivers/char/agp/generic.c |  8 +
 drivers/gpu/drm/nouveau/nouveau_gem.c  |  4 +--
 drivers/md/bcache/util.h   | 12 ++--
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h|  3 --
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--
 drivers/net/ethernet/chelsio/cxgb3/l2t.c   |  2 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 31 
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |  9 ++
 drivers/net/ethernet/mellanox/mlx4/mr.c|  9 ++
 drivers/nvdimm/dimm_devs.c |  5 +---
 .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +--
 drivers/xen/evtchn.c   | 14 +
 fs/btrfs/ctree.c   |  9 ++
 fs/btrfs/ioctl.c   |  9 ++
 fs/btrfs/send.c| 27 ++---
 fs/ceph/file.c |  9 ++
 fs/select.c|  5 +---
 fs/xattr.c | 27 ++---
 kernel/bpf/hashtab.c   | 11 ++-
 lib/iov_iter.c |  5 +---
 mm/frame_vector.c  |  5 +---
 net/ipv4/inet_hashtables.c |  6 +---
 net/ipv4/tcp_metrics.c |  5 +---
 net/mpls/af_mpls.c |  5 +---
 net/netfilter/x_tables.c   | 34 ++
 net/netfilter/xt_recent.c  |  5 +---
 net/sched/sch_choke.c  |  5 +---
 net/sched/sch_fq_codel.c   | 26 -
 net/sched/sch_hhf.c| 33 ++---
 net/sched/sch_netem.c  |  6 +---
 net/sched/sch_sfq.c|  6 +---
 security/keys/keyctl.c | 22 --
 35 files changed, 96 insertions(+), 319 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 4f74511015b8..e6bbb33d2956 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct 
kvm_s390_skeys *args)
if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

Ilya has noticed that I've screwed up some k[zc]alloc conversions and
didn't use the kvzalloc. This is an updated patch with some acks
collected on the way
---
>From a7b89c6d0a3c685045e37740c8f97b065f37e0a4 Mon Sep 17 00:00:00 2001
From: Michal Hocko 
Date: Wed, 4 Jan 2017 13:30:32 +0100
Subject: [PATCH] treewide: use kv[mz]alloc* rather than opencoded variants

There are many code paths opencoding kvmalloc. Let's use the helper
instead. The main difference to kvmalloc is that those users are usually
not considering all the aspects of the memory allocator. E.g. allocation
requests < 64kB are basically never failing and invoke OOM killer to
satisfy the allocation. This sounds too disruptive for something that
has a reasonable fallback - the vmalloc. On the other hand those
requests might fallback to vmalloc even when the memory allocator would
succeed after several more reclaim/compaction attempts previously. There
is no guarantee something like that happens though.

This patch converts many of those places to kv[mz]alloc* helpers because
they are more conservative.

Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
Cc: Herbert Xu 
Cc: Anton Vorontsov 
Cc: Colin Cross 
Cc: Kees Cook 
Cc: Tony Luck 
Cc: "Rafael J. Wysocki" 
Cc: Ben Skeggs 
Cc: Kent Overstreet 
Cc: Santosh Raspatur 
Cc: Hariprasad S 
Cc: Tariq Toukan 
Cc: Yishai Hadas 
Cc: Oleg Drokin 
Cc: Andreas Dilger 
Cc: Boris Ostrovsky 
Cc: "Yan, Zheng" 
Cc: Alexander Viro 
Cc: Alexei Starovoitov 
Cc: Eric Dumazet 
Cc: net...@vger.kernel.org
Acked-by: Christian Borntraeger  # KVM/s390
Acked-by: Dan Williams  # nvdim
Acked-by: David Sterba  # btrfs
Acked-by: Ilya Dryomov  # Ceph
Signed-off-by: Michal Hocko 
---
 arch/s390/kvm/kvm-s390.c   | 10 ++-
 crypto/lzo.c   |  4 +--
 drivers/acpi/apei/erst.c   |  8 ++---
 drivers/char/agp/generic.c |  8 +
 drivers/gpu/drm/nouveau/nouveau_gem.c  |  4 +--
 drivers/md/bcache/util.h   | 12 ++--
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h|  3 --
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--
 drivers/net/ethernet/chelsio/cxgb3/l2t.c   |  2 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 31 
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |  9 ++
 drivers/net/ethernet/mellanox/mlx4/mr.c|  9 ++
 drivers/nvdimm/dimm_devs.c |  5 +---
 .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +--
 drivers/xen/evtchn.c   | 14 +
 fs/btrfs/ctree.c   |  9 ++
 fs/btrfs/ioctl.c   |  9 ++
 fs/btrfs/send.c| 27 ++---
 fs/ceph/file.c |  9 ++
 fs/select.c|  5 +---
 fs/xattr.c | 27 ++---
 kernel/bpf/hashtab.c   | 11 ++-
 lib/iov_iter.c |  5 +---
 mm/frame_vector.c  |  5 +---
 net/ipv4/inet_hashtables.c |  6 +---
 net/ipv4/tcp_metrics.c |  5 +---
 net/mpls/af_mpls.c |  5 +---
 net/netfilter/x_tables.c   | 34 ++
 net/netfilter/xt_recent.c  |  5 +---
 net/sched/sch_choke.c  |  5 +---
 net/sched/sch_fq_codel.c   | 26 -
 net/sched/sch_hhf.c| 33 ++---
 net/sched/sch_netem.c  |  6 +---
 net/sched/sch_sfq.c|  6 +---
 security/keys/keyctl.c | 22 --
 35 files changed, 96 insertions(+), 319 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 4f74511015b8..e6bbb33d2956 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct 
kvm_s390_skeys *args)
if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
return -EINVAL;
 
-   keys = kmalloc_array(args->count, sizeof(uint8_t),
-GFP_KERNEL | __GFP_NOWARN);
-   if (!keys)
-   keys = vmalloc(sizeof(uint8_t) * args->count);
+   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
if (!keys)
return -ENOMEM;
 
@@ -1171,10 +1168,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct 
kvm_s390_skeys *args)
if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
return -EINVAL;
 
-   keys = kmalloc_array(args->count, sizeof(uint8_t),
-  

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Kees Cook]

On Thu, Jan 12, 2017 at 7:37 AM, Michal Hocko  wrote:
> From: Michal Hocko 
>
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
>
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.
>
> Cc: Martin Schwidefsky 
> Cc: Heiko Carstens 
> Cc: Herbert Xu 
> Cc: Anton Vorontsov 
> Cc: Colin Cross 
> Cc: Kees Cook 
> Cc: Tony Luck 
> Cc: "Rafael J. Wysocki" 
> Cc: Ben Skeggs 
> Cc: Kent Overstreet 
> Cc: Santosh Raspatur 
> Cc: Hariprasad S 
> Cc: Tariq Toukan 
> Cc: Yishai Hadas 
> Cc: Dan Williams 
> Cc: Oleg Drokin 
> Cc: Andreas Dilger 
> Cc: Boris Ostrovsky 
> Cc: David Sterba 
> Cc: "Yan, Zheng" 
> Cc: Ilya Dryomov 
> Cc: Alexander Viro 
> Cc: Alexei Starovoitov 
> Cc: Eric Dumazet 
> Cc: net...@vger.kernel.org
> Signed-off-by: Michal Hocko 
> ---
>  arch/s390/kvm/kvm-s390.c   | 10 ++-
>  crypto/lzo.c   |  4 +--
>  drivers/acpi/apei/erst.c   |  8 ++---
>  drivers/char/agp/generic.c |  8 +
>  drivers/gpu/drm/nouveau/nouveau_gem.c  |  4 +--
>  drivers/md/bcache/util.h   | 12 ++--
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h|  3 --
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--
>  drivers/net/ethernet/chelsio/cxgb3/l2t.c   |  2 +-
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 31 
>  drivers/net/ethernet/mellanox/mlx4/en_tx.c |  9 ++
>  drivers/net/ethernet/mellanox/mlx4/mr.c|  9 ++
>  drivers/nvdimm/dimm_devs.c |  5 +---
>  .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +--
>  drivers/xen/evtchn.c   | 14 +
>  fs/btrfs/ctree.c   |  9 ++
>  fs/btrfs/ioctl.c   |  9 ++
>  fs/btrfs/send.c| 27 ++---
>  fs/ceph/file.c |  9 ++
>  fs/select.c|  5 +---
>  fs/xattr.c | 27 ++---
>  kernel/bpf/hashtab.c   | 11 ++-
>  lib/iov_iter.c |  5 +---
>  mm/frame_vector.c  |  5 +---
>  net/ipv4/inet_hashtables.c |  6 +---
>  net/ipv4/tcp_metrics.c |  5 +---
>  net/mpls/af_mpls.c |  5 +---
>  net/netfilter/x_tables.c   | 34 
> ++
>  net/netfilter/xt_recent.c  |  5 +---
>  net/sched/sch_choke.c  |  5 +---
>  net/sched/sch_fq_codel.c   | 26 -
>  net/sched/sch_hhf.c| 33 ++---
>  net/sched/sch_netem.c  |  6 +---
>  net/sched/sch_sfq.c|  6 +---
>  security/keys/keyctl.c | 22 --
>  35 files changed, 96 insertions(+), 319 deletions(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 4f74511015b8..e6bbb33d2956 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct 
> kvm_s390_skeys *args)
> if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
> return -EINVAL;
>
> -   keys = kmalloc_array(args->count, sizeof(uint8_t),
> -GFP_KERNEL | __GFP_NOWARN);
> -   if (!keys)
> -   keys = vmalloc(sizeof(uint8_t) * 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Kees Cook]

On Thu, Jan 12, 2017 at 7:37 AM, Michal Hocko  wrote:
> From: Michal Hocko 
>
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
>
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.
>
> Cc: Martin Schwidefsky 
> Cc: Heiko Carstens 
> Cc: Herbert Xu 
> Cc: Anton Vorontsov 
> Cc: Colin Cross 
> Cc: Kees Cook 
> Cc: Tony Luck 
> Cc: "Rafael J. Wysocki" 
> Cc: Ben Skeggs 
> Cc: Kent Overstreet 
> Cc: Santosh Raspatur 
> Cc: Hariprasad S 
> Cc: Tariq Toukan 
> Cc: Yishai Hadas 
> Cc: Dan Williams 
> Cc: Oleg Drokin 
> Cc: Andreas Dilger 
> Cc: Boris Ostrovsky 
> Cc: David Sterba 
> Cc: "Yan, Zheng" 
> Cc: Ilya Dryomov 
> Cc: Alexander Viro 
> Cc: Alexei Starovoitov 
> Cc: Eric Dumazet 
> Cc: net...@vger.kernel.org
> Signed-off-by: Michal Hocko 
> ---
>  arch/s390/kvm/kvm-s390.c   | 10 ++-
>  crypto/lzo.c   |  4 +--
>  drivers/acpi/apei/erst.c   |  8 ++---
>  drivers/char/agp/generic.c |  8 +
>  drivers/gpu/drm/nouveau/nouveau_gem.c  |  4 +--
>  drivers/md/bcache/util.h   | 12 ++--
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h|  3 --
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--
>  drivers/net/ethernet/chelsio/cxgb3/l2t.c   |  2 +-
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 31 
>  drivers/net/ethernet/mellanox/mlx4/en_tx.c |  9 ++
>  drivers/net/ethernet/mellanox/mlx4/mr.c|  9 ++
>  drivers/nvdimm/dimm_devs.c |  5 +---
>  .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +--
>  drivers/xen/evtchn.c   | 14 +
>  fs/btrfs/ctree.c   |  9 ++
>  fs/btrfs/ioctl.c   |  9 ++
>  fs/btrfs/send.c| 27 ++---
>  fs/ceph/file.c |  9 ++
>  fs/select.c|  5 +---
>  fs/xattr.c | 27 ++---
>  kernel/bpf/hashtab.c   | 11 ++-
>  lib/iov_iter.c |  5 +---
>  mm/frame_vector.c  |  5 +---
>  net/ipv4/inet_hashtables.c |  6 +---
>  net/ipv4/tcp_metrics.c |  5 +---
>  net/mpls/af_mpls.c |  5 +---
>  net/netfilter/x_tables.c   | 34 
> ++
>  net/netfilter/xt_recent.c  |  5 +---
>  net/sched/sch_choke.c  |  5 +---
>  net/sched/sch_fq_codel.c   | 26 -
>  net/sched/sch_hhf.c| 33 ++---
>  net/sched/sch_netem.c  |  6 +---
>  net/sched/sch_sfq.c|  6 +---
>  security/keys/keyctl.c | 22 --
>  35 files changed, 96 insertions(+), 319 deletions(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 4f74511015b8..e6bbb33d2956 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct 
> kvm_s390_skeys *args)
> if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
> return -EINVAL;
>
> -   keys = kmalloc_array(args->count, sizeof(uint8_t),
> -GFP_KERNEL | __GFP_NOWARN);
> -   if (!keys)
> -   keys = vmalloc(sizeof(uint8_t) * args->count);
> +   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);

Before doing this conversion, can we add a kvmalloc_array() API? This
conversion could allow for the reintroduction of integer overflow
flaws. (This particular situation isn't at risk since ->count is
checked, but I'd prefer we not create a risky set of examples for
using kvmalloc.)

Besides that: yes please. Less open coding. :)

-Kees

-- 
Kees Cook
Nexus Security

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

On Thu 12-01-17 17:54:34, Ilya Dryomov wrote:
> On Thu, Jan 12, 2017 at 4:37 PM, Michal Hocko  wrote:
> > From: Michal Hocko 
> >
> > There are many code paths opencoding kvmalloc. Let's use the helper
> > instead. The main difference to kvmalloc is that those users are usually
> > not considering all the aspects of the memory allocator. E.g. allocation
> > requests < 64kB are basically never failing and invoke OOM killer to
> > satisfy the allocation. This sounds too disruptive for something that
> > has a reasonable fallback - the vmalloc. On the other hand those
> > requests might fallback to vmalloc even when the memory allocator would
> > succeed after several more reclaim/compaction attempts previously. There
> > is no guarantee something like that happens though.
> >
> > This patch converts many of those places to kv[mz]alloc* helpers because
> > they are more conservative.
> >
> > Cc: Martin Schwidefsky 
> > Cc: Heiko Carstens 
> > Cc: Herbert Xu 
> > Cc: Anton Vorontsov 
> > Cc: Colin Cross 
> > Cc: Kees Cook 
> > Cc: Tony Luck 
> > Cc: "Rafael J. Wysocki" 
> > Cc: Ben Skeggs 
> > Cc: Kent Overstreet 
> > Cc: Santosh Raspatur 
> > Cc: Hariprasad S 
> > Cc: Tariq Toukan 
> > Cc: Yishai Hadas 
> > Cc: Dan Williams 
> > Cc: Oleg Drokin 
> > Cc: Andreas Dilger 
> > Cc: Boris Ostrovsky 
> > Cc: David Sterba 
> > Cc: "Yan, Zheng" 
> > Cc: Ilya Dryomov 
> > Cc: Alexander Viro 
> > Cc: Alexei Starovoitov 
> > Cc: Eric Dumazet 
> > Cc: net...@vger.kernel.org
> > Signed-off-by: Michal Hocko 
> > ---
> >  arch/s390/kvm/kvm-s390.c   | 10 ++-
> >  crypto/lzo.c   |  4 +--
> >  drivers/acpi/apei/erst.c   |  8 ++---
> >  drivers/char/agp/generic.c |  8 +
> >  drivers/gpu/drm/nouveau/nouveau_gem.c  |  4 +--
> >  drivers/md/bcache/util.h   | 12 ++--
> >  drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h|  3 --
> >  drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--
> >  drivers/net/ethernet/chelsio/cxgb3/l2t.c   |  2 +-
> >  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 31 
> > 
> >  drivers/net/ethernet/mellanox/mlx4/en_tx.c |  9 ++
> >  drivers/net/ethernet/mellanox/mlx4/mr.c|  9 ++
> >  drivers/nvdimm/dimm_devs.c |  5 +---
> >  .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +--
> >  drivers/xen/evtchn.c   | 14 +
> >  fs/btrfs/ctree.c   |  9 ++
> >  fs/btrfs/ioctl.c   |  9 ++
> >  fs/btrfs/send.c| 27 ++---
> >  fs/ceph/file.c |  9 ++
> >  fs/select.c|  5 +---
> >  fs/xattr.c | 27 ++---
> >  kernel/bpf/hashtab.c   | 11 ++-
> >  lib/iov_iter.c |  5 +---
> >  mm/frame_vector.c  |  5 +---
> >  net/ipv4/inet_hashtables.c |  6 +---
> >  net/ipv4/tcp_metrics.c |  5 +---
> >  net/mpls/af_mpls.c |  5 +---
> >  net/netfilter/x_tables.c   | 34 
> > ++
> >  net/netfilter/xt_recent.c  |  5 +---
> >  net/sched/sch_choke.c  |  5 +---
> >  net/sched/sch_fq_codel.c   | 26 -
> >  net/sched/sch_hhf.c| 33 
> > ++---
> >  net/sched/sch_netem.c  |  6 +---
> >  net/sched/sch_sfq.c|  6 +---
> >  security/keys/keyctl.c | 22 --
> >  35 files changed, 96 insertions(+), 319 deletions(-)
> >
> > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> > index 4f74511015b8..e6bbb33d2956 100644
> > --- a/arch/s390/kvm/kvm-s390.c
> > +++ b/arch/s390/kvm/kvm-s390.c
> > @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, 
> > struct kvm_s390_skeys *args)
> > if (args->count < 1 || args->count > 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

On Thu 12-01-17 17:54:34, Ilya Dryomov wrote:
> On Thu, Jan 12, 2017 at 4:37 PM, Michal Hocko  wrote:
> > From: Michal Hocko 
> >
> > There are many code paths opencoding kvmalloc. Let's use the helper
> > instead. The main difference to kvmalloc is that those users are usually
> > not considering all the aspects of the memory allocator. E.g. allocation
> > requests < 64kB are basically never failing and invoke OOM killer to
> > satisfy the allocation. This sounds too disruptive for something that
> > has a reasonable fallback - the vmalloc. On the other hand those
> > requests might fallback to vmalloc even when the memory allocator would
> > succeed after several more reclaim/compaction attempts previously. There
> > is no guarantee something like that happens though.
> >
> > This patch converts many of those places to kv[mz]alloc* helpers because
> > they are more conservative.
> >
> > Cc: Martin Schwidefsky 
> > Cc: Heiko Carstens 
> > Cc: Herbert Xu 
> > Cc: Anton Vorontsov 
> > Cc: Colin Cross 
> > Cc: Kees Cook 
> > Cc: Tony Luck 
> > Cc: "Rafael J. Wysocki" 
> > Cc: Ben Skeggs 
> > Cc: Kent Overstreet 
> > Cc: Santosh Raspatur 
> > Cc: Hariprasad S 
> > Cc: Tariq Toukan 
> > Cc: Yishai Hadas 
> > Cc: Dan Williams 
> > Cc: Oleg Drokin 
> > Cc: Andreas Dilger 
> > Cc: Boris Ostrovsky 
> > Cc: David Sterba 
> > Cc: "Yan, Zheng" 
> > Cc: Ilya Dryomov 
> > Cc: Alexander Viro 
> > Cc: Alexei Starovoitov 
> > Cc: Eric Dumazet 
> > Cc: net...@vger.kernel.org
> > Signed-off-by: Michal Hocko 
> > ---
> >  arch/s390/kvm/kvm-s390.c   | 10 ++-
> >  crypto/lzo.c   |  4 +--
> >  drivers/acpi/apei/erst.c   |  8 ++---
> >  drivers/char/agp/generic.c |  8 +
> >  drivers/gpu/drm/nouveau/nouveau_gem.c  |  4 +--
> >  drivers/md/bcache/util.h   | 12 ++--
> >  drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h|  3 --
> >  drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--
> >  drivers/net/ethernet/chelsio/cxgb3/l2t.c   |  2 +-
> >  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 31 
> > 
> >  drivers/net/ethernet/mellanox/mlx4/en_tx.c |  9 ++
> >  drivers/net/ethernet/mellanox/mlx4/mr.c|  9 ++
> >  drivers/nvdimm/dimm_devs.c |  5 +---
> >  .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +--
> >  drivers/xen/evtchn.c   | 14 +
> >  fs/btrfs/ctree.c   |  9 ++
> >  fs/btrfs/ioctl.c   |  9 ++
> >  fs/btrfs/send.c| 27 ++---
> >  fs/ceph/file.c |  9 ++
> >  fs/select.c|  5 +---
> >  fs/xattr.c | 27 ++---
> >  kernel/bpf/hashtab.c   | 11 ++-
> >  lib/iov_iter.c |  5 +---
> >  mm/frame_vector.c  |  5 +---
> >  net/ipv4/inet_hashtables.c |  6 +---
> >  net/ipv4/tcp_metrics.c |  5 +---
> >  net/mpls/af_mpls.c |  5 +---
> >  net/netfilter/x_tables.c   | 34 
> > ++
> >  net/netfilter/xt_recent.c  |  5 +---
> >  net/sched/sch_choke.c  |  5 +---
> >  net/sched/sch_fq_codel.c   | 26 -
> >  net/sched/sch_hhf.c| 33 
> > ++---
> >  net/sched/sch_netem.c  |  6 +---
> >  net/sched/sch_sfq.c|  6 +---
> >  security/keys/keyctl.c | 22 --
> >  35 files changed, 96 insertions(+), 319 deletions(-)
> >
> > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> > index 4f74511015b8..e6bbb33d2956 100644
> > --- a/arch/s390/kvm/kvm-s390.c
> > +++ b/arch/s390/kvm/kvm-s390.c
> > @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, 
> > struct kvm_s390_skeys *args)
> > if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
> > return -EINVAL;
> >
> > -   keys = kmalloc_array(args->count, sizeof(uint8_t),
> > -GFP_KERNEL | __GFP_NOWARN);
> > -   if (!keys)
> > -   keys = vmalloc(sizeof(uint8_t) * args->count);
> > +   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
> > if (!keys)
> > return -ENOMEM;
> >
> > @@ -1171,10 +1168,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, 
> > struct kvm_s390_skeys *args)
> > if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
> > 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Dan Williams]

On Thu, Jan 12, 2017 at 7:37 AM, Michal Hocko  wrote:
> From: Michal Hocko 
>
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
>
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.
>
[..]
> Cc: Dan Williams 
[..]
>  drivers/nvdimm/dimm_devs.c |  5 +---

Acked-by: Dan Williams 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Dan Williams]

On Thu, Jan 12, 2017 at 7:37 AM, Michal Hocko  wrote:
> From: Michal Hocko 
>
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
>
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.
>
[..]
> Cc: Dan Williams 
[..]
>  drivers/nvdimm/dimm_devs.c |  5 +---

Acked-by: Dan Williams 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Ilya Dryomov]

On Thu, Jan 12, 2017 at 4:37 PM, Michal Hocko  wrote:
> From: Michal Hocko 
>
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
>
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.
>
> Cc: Martin Schwidefsky 
> Cc: Heiko Carstens 
> Cc: Herbert Xu 
> Cc: Anton Vorontsov 
> Cc: Colin Cross 
> Cc: Kees Cook 
> Cc: Tony Luck 
> Cc: "Rafael J. Wysocki" 
> Cc: Ben Skeggs 
> Cc: Kent Overstreet 
> Cc: Santosh Raspatur 
> Cc: Hariprasad S 
> Cc: Tariq Toukan 
> Cc: Yishai Hadas 
> Cc: Dan Williams 
> Cc: Oleg Drokin 
> Cc: Andreas Dilger 
> Cc: Boris Ostrovsky 
> Cc: David Sterba 
> Cc: "Yan, Zheng" 
> Cc: Ilya Dryomov 
> Cc: Alexander Viro 
> Cc: Alexei Starovoitov 
> Cc: Eric Dumazet 
> Cc: net...@vger.kernel.org
> Signed-off-by: Michal Hocko 
> ---
>  arch/s390/kvm/kvm-s390.c   | 10 ++-
>  crypto/lzo.c   |  4 +--
>  drivers/acpi/apei/erst.c   |  8 ++---
>  drivers/char/agp/generic.c |  8 +
>  drivers/gpu/drm/nouveau/nouveau_gem.c  |  4 +--
>  drivers/md/bcache/util.h   | 12 ++--
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h|  3 --
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--
>  drivers/net/ethernet/chelsio/cxgb3/l2t.c   |  2 +-
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 31 
>  drivers/net/ethernet/mellanox/mlx4/en_tx.c |  9 ++
>  drivers/net/ethernet/mellanox/mlx4/mr.c|  9 ++
>  drivers/nvdimm/dimm_devs.c |  5 +---
>  .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +--
>  drivers/xen/evtchn.c   | 14 +
>  fs/btrfs/ctree.c   |  9 ++
>  fs/btrfs/ioctl.c   |  9 ++
>  fs/btrfs/send.c| 27 ++---
>  fs/ceph/file.c |  9 ++
>  fs/select.c|  5 +---
>  fs/xattr.c | 27 ++---
>  kernel/bpf/hashtab.c   | 11 ++-
>  lib/iov_iter.c |  5 +---
>  mm/frame_vector.c  |  5 +---
>  net/ipv4/inet_hashtables.c |  6 +---
>  net/ipv4/tcp_metrics.c |  5 +---
>  net/mpls/af_mpls.c |  5 +---
>  net/netfilter/x_tables.c   | 34 
> ++
>  net/netfilter/xt_recent.c  |  5 +---
>  net/sched/sch_choke.c  |  5 +---
>  net/sched/sch_fq_codel.c   | 26 -
>  net/sched/sch_hhf.c| 33 ++---
>  net/sched/sch_netem.c  |  6 +---
>  net/sched/sch_sfq.c|  6 +---
>  security/keys/keyctl.c | 22 --
>  35 files changed, 96 insertions(+), 319 deletions(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 4f74511015b8..e6bbb33d2956 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct 
> kvm_s390_skeys *args)
> if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
> return -EINVAL;
>
> -   keys = kmalloc_array(args->count, sizeof(uint8_t),
> -GFP_KERNEL | __GFP_NOWARN);
> -   if (!keys)
> -   keys = vmalloc(sizeof(uint8_t) * 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Ilya Dryomov]

On Thu, Jan 12, 2017 at 4:37 PM, Michal Hocko  wrote:
> From: Michal Hocko 
>
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
>
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.
>
> Cc: Martin Schwidefsky 
> Cc: Heiko Carstens 
> Cc: Herbert Xu 
> Cc: Anton Vorontsov 
> Cc: Colin Cross 
> Cc: Kees Cook 
> Cc: Tony Luck 
> Cc: "Rafael J. Wysocki" 
> Cc: Ben Skeggs 
> Cc: Kent Overstreet 
> Cc: Santosh Raspatur 
> Cc: Hariprasad S 
> Cc: Tariq Toukan 
> Cc: Yishai Hadas 
> Cc: Dan Williams 
> Cc: Oleg Drokin 
> Cc: Andreas Dilger 
> Cc: Boris Ostrovsky 
> Cc: David Sterba 
> Cc: "Yan, Zheng" 
> Cc: Ilya Dryomov 
> Cc: Alexander Viro 
> Cc: Alexei Starovoitov 
> Cc: Eric Dumazet 
> Cc: net...@vger.kernel.org
> Signed-off-by: Michal Hocko 
> ---
>  arch/s390/kvm/kvm-s390.c   | 10 ++-
>  crypto/lzo.c   |  4 +--
>  drivers/acpi/apei/erst.c   |  8 ++---
>  drivers/char/agp/generic.c |  8 +
>  drivers/gpu/drm/nouveau/nouveau_gem.c  |  4 +--
>  drivers/md/bcache/util.h   | 12 ++--
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h|  3 --
>  drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--
>  drivers/net/ethernet/chelsio/cxgb3/l2t.c   |  2 +-
>  drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 31 
>  drivers/net/ethernet/mellanox/mlx4/en_tx.c |  9 ++
>  drivers/net/ethernet/mellanox/mlx4/mr.c|  9 ++
>  drivers/nvdimm/dimm_devs.c |  5 +---
>  .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +--
>  drivers/xen/evtchn.c   | 14 +
>  fs/btrfs/ctree.c   |  9 ++
>  fs/btrfs/ioctl.c   |  9 ++
>  fs/btrfs/send.c| 27 ++---
>  fs/ceph/file.c |  9 ++
>  fs/select.c|  5 +---
>  fs/xattr.c | 27 ++---
>  kernel/bpf/hashtab.c   | 11 ++-
>  lib/iov_iter.c |  5 +---
>  mm/frame_vector.c  |  5 +---
>  net/ipv4/inet_hashtables.c |  6 +---
>  net/ipv4/tcp_metrics.c |  5 +---
>  net/mpls/af_mpls.c |  5 +---
>  net/netfilter/x_tables.c   | 34 
> ++
>  net/netfilter/xt_recent.c  |  5 +---
>  net/sched/sch_choke.c  |  5 +---
>  net/sched/sch_fq_codel.c   | 26 -
>  net/sched/sch_hhf.c| 33 ++---
>  net/sched/sch_netem.c  |  6 +---
>  net/sched/sch_sfq.c|  6 +---
>  security/keys/keyctl.c | 22 --
>  35 files changed, 96 insertions(+), 319 deletions(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 4f74511015b8..e6bbb33d2956 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct 
> kvm_s390_skeys *args)
> if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
> return -EINVAL;
>
> -   keys = kmalloc_array(args->count, sizeof(uint8_t),
> -GFP_KERNEL | __GFP_NOWARN);
> -   if (!keys)
> -   keys = vmalloc(sizeof(uint8_t) * args->count);
> +   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
> if (!keys)
> return -ENOMEM;
>
> @@ -1171,10 +1168,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct 
> kvm_s390_skeys *args)
> if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
> return -EINVAL;
>
> -   keys = kmalloc_array(args->count, sizeof(uint8_t),
> -GFP_KERNEL | __GFP_NOWARN);
> -   if (!keys)
> -   keys = vmalloc(sizeof(uint8_t) * args->count);
> +   keys = 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Christian Borntraeger]

On 01/12/2017 04:37 PM, Michal Hocko wrote:
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 4f74511015b8..e6bbb33d2956 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct 
> kvm_s390_skeys *args)
>   if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
>   return -EINVAL;
> 
> - keys = kmalloc_array(args->count, sizeof(uint8_t),
> -  GFP_KERNEL | __GFP_NOWARN);
> - if (!keys)
> - keys = vmalloc(sizeof(uint8_t) * args->count);
> + keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
>   if (!keys)
>   return -ENOMEM;
> 
> @@ -1171,10 +1168,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct 
> kvm_s390_skeys *args)
>   if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
>   return -EINVAL;
> 
> - keys = kmalloc_array(args->count, sizeof(uint8_t),
> -  GFP_KERNEL | __GFP_NOWARN);
> - if (!keys)
> - keys = vmalloc(sizeof(uint8_t) * args->count);
> + keys = kvmalloc(sizeof(uint8_t) * args->count, GFP_KERNEL);
>   if (!keys)
>   return -ENOMEM;

KVM/s390 parts

Acked-by: Christian Borntraeger 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Christian Borntraeger]

On 01/12/2017 04:37 PM, Michal Hocko wrote:
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 4f74511015b8..e6bbb33d2956 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct 
> kvm_s390_skeys *args)
>   if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
>   return -EINVAL;
> 
> - keys = kmalloc_array(args->count, sizeof(uint8_t),
> -  GFP_KERNEL | __GFP_NOWARN);
> - if (!keys)
> - keys = vmalloc(sizeof(uint8_t) * args->count);
> + keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
>   if (!keys)
>   return -ENOMEM;
> 
> @@ -1171,10 +1168,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct 
> kvm_s390_skeys *args)
>   if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
>   return -EINVAL;
> 
> - keys = kmalloc_array(args->count, sizeof(uint8_t),
> -  GFP_KERNEL | __GFP_NOWARN);
> - if (!keys)
> - keys = vmalloc(sizeof(uint8_t) * args->count);
> + keys = kvmalloc(sizeof(uint8_t) * args->count, GFP_KERNEL);
>   if (!keys)
>   return -ENOMEM;

KVM/s390 parts

Acked-by: Christian Borntraeger 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[David Sterba]

On Thu, Jan 12, 2017 at 04:37:16PM +0100, Michal Hocko wrote:
> From: Michal Hocko 
> 
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
> 
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.

For the btrfs bits,

Acked-by: David Sterba 

Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[David Sterba]

On Thu, Jan 12, 2017 at 04:37:16PM +0100, Michal Hocko wrote:
> From: Michal Hocko 
> 
> There are many code paths opencoding kvmalloc. Let's use the helper
> instead. The main difference to kvmalloc is that those users are usually
> not considering all the aspects of the memory allocator. E.g. allocation
> requests < 64kB are basically never failing and invoke OOM killer to
> satisfy the allocation. This sounds too disruptive for something that
> has a reasonable fallback - the vmalloc. On the other hand those
> requests might fallback to vmalloc even when the memory allocator would
> succeed after several more reclaim/compaction attempts previously. There
> is no guarantee something like that happens though.
> 
> This patch converts many of those places to kv[mz]alloc* helpers because
> they are more conservative.

For the btrfs bits,

Acked-by: David Sterba 

[PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

From: Michal Hocko 

There are many code paths opencoding kvmalloc. Let's use the helper
instead. The main difference to kvmalloc is that those users are usually
not considering all the aspects of the memory allocator. E.g. allocation
requests < 64kB are basically never failing and invoke OOM killer to
satisfy the allocation. This sounds too disruptive for something that
has a reasonable fallback - the vmalloc. On the other hand those
requests might fallback to vmalloc even when the memory allocator would
succeed after several more reclaim/compaction attempts previously. There
is no guarantee something like that happens though.

This patch converts many of those places to kv[mz]alloc* helpers because
they are more conservative.

Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
Cc: Herbert Xu 
Cc: Anton Vorontsov 
Cc: Colin Cross 
Cc: Kees Cook 
Cc: Tony Luck 
Cc: "Rafael J. Wysocki" 
Cc: Ben Skeggs 
Cc: Kent Overstreet 
Cc: Santosh Raspatur 
Cc: Hariprasad S 
Cc: Tariq Toukan 
Cc: Yishai Hadas 
Cc: Dan Williams 
Cc: Oleg Drokin 
Cc: Andreas Dilger 
Cc: Boris Ostrovsky 
Cc: David Sterba 
Cc: "Yan, Zheng" 
Cc: Ilya Dryomov 
Cc: Alexander Viro 
Cc: Alexei Starovoitov 
Cc: Eric Dumazet 
Cc: net...@vger.kernel.org
Signed-off-by: Michal Hocko 
---
 arch/s390/kvm/kvm-s390.c   | 10 ++-
 crypto/lzo.c   |  4 +--
 drivers/acpi/apei/erst.c   |  8 ++---
 drivers/char/agp/generic.c |  8 +
 drivers/gpu/drm/nouveau/nouveau_gem.c  |  4 +--
 drivers/md/bcache/util.h   | 12 ++--
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h|  3 --
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--
 drivers/net/ethernet/chelsio/cxgb3/l2t.c   |  2 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 31 
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |  9 ++
 drivers/net/ethernet/mellanox/mlx4/mr.c|  9 ++
 drivers/nvdimm/dimm_devs.c |  5 +---
 .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +--
 drivers/xen/evtchn.c   | 14 +
 fs/btrfs/ctree.c   |  9 ++
 fs/btrfs/ioctl.c   |  9 ++
 fs/btrfs/send.c| 27 ++---
 fs/ceph/file.c |  9 ++
 fs/select.c|  5 +---
 fs/xattr.c | 27 ++---
 kernel/bpf/hashtab.c   | 11 ++-
 lib/iov_iter.c |  5 +---
 mm/frame_vector.c  |  5 +---
 net/ipv4/inet_hashtables.c |  6 +---
 net/ipv4/tcp_metrics.c |  5 +---
 net/mpls/af_mpls.c |  5 +---
 net/netfilter/x_tables.c   | 34 ++
 net/netfilter/xt_recent.c  |  5 +---
 net/sched/sch_choke.c  |  5 +---
 net/sched/sch_fq_codel.c   | 26 -
 net/sched/sch_hhf.c| 33 ++---
 net/sched/sch_netem.c  |  6 +---
 net/sched/sch_sfq.c|  6 +---
 security/keys/keyctl.c | 22 --
 35 files changed, 96 insertions(+), 319 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 4f74511015b8..e6bbb33d2956 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct 
kvm_s390_skeys *args)
if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
return -EINVAL;
 
-   keys = kmalloc_array(args->count, sizeof(uint8_t),
-GFP_KERNEL | __GFP_NOWARN);
-   if (!keys)
-   keys = vmalloc(sizeof(uint8_t) * args->count);
+   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
if (!keys)
return -ENOMEM;
 
@@ -1171,10 +1168,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct 
kvm_s390_skeys *args)
if 

[PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants

[Michal Hocko]

From: Michal Hocko 

There are many code paths opencoding kvmalloc. Let's use the helper
instead. The main difference to kvmalloc is that those users are usually
not considering all the aspects of the memory allocator. E.g. allocation
requests < 64kB are basically never failing and invoke OOM killer to
satisfy the allocation. This sounds too disruptive for something that
has a reasonable fallback - the vmalloc. On the other hand those
requests might fallback to vmalloc even when the memory allocator would
succeed after several more reclaim/compaction attempts previously. There
is no guarantee something like that happens though.

This patch converts many of those places to kv[mz]alloc* helpers because
they are more conservative.

Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
Cc: Herbert Xu 
Cc: Anton Vorontsov 
Cc: Colin Cross 
Cc: Kees Cook 
Cc: Tony Luck 
Cc: "Rafael J. Wysocki" 
Cc: Ben Skeggs 
Cc: Kent Overstreet 
Cc: Santosh Raspatur 
Cc: Hariprasad S 
Cc: Tariq Toukan 
Cc: Yishai Hadas 
Cc: Dan Williams 
Cc: Oleg Drokin 
Cc: Andreas Dilger 
Cc: Boris Ostrovsky 
Cc: David Sterba 
Cc: "Yan, Zheng" 
Cc: Ilya Dryomov 
Cc: Alexander Viro 
Cc: Alexei Starovoitov 
Cc: Eric Dumazet 
Cc: net...@vger.kernel.org
Signed-off-by: Michal Hocko 
---
 arch/s390/kvm/kvm-s390.c   | 10 ++-
 crypto/lzo.c   |  4 +--
 drivers/acpi/apei/erst.c   |  8 ++---
 drivers/char/agp/generic.c |  8 +
 drivers/gpu/drm/nouveau/nouveau_gem.c  |  4 +--
 drivers/md/bcache/util.h   | 12 ++--
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_defs.h|  3 --
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 25 ++--
 drivers/net/ethernet/chelsio/cxgb3/l2t.c   |  2 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c| 31 
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |  9 ++
 drivers/net/ethernet/mellanox/mlx4/mr.c|  9 ++
 drivers/nvdimm/dimm_devs.c |  5 +---
 .../staging/lustre/lnet/libcfs/linux/linux-mem.c   | 11 +--
 drivers/xen/evtchn.c   | 14 +
 fs/btrfs/ctree.c   |  9 ++
 fs/btrfs/ioctl.c   |  9 ++
 fs/btrfs/send.c| 27 ++---
 fs/ceph/file.c |  9 ++
 fs/select.c|  5 +---
 fs/xattr.c | 27 ++---
 kernel/bpf/hashtab.c   | 11 ++-
 lib/iov_iter.c |  5 +---
 mm/frame_vector.c  |  5 +---
 net/ipv4/inet_hashtables.c |  6 +---
 net/ipv4/tcp_metrics.c |  5 +---
 net/mpls/af_mpls.c |  5 +---
 net/netfilter/x_tables.c   | 34 ++
 net/netfilter/xt_recent.c  |  5 +---
 net/sched/sch_choke.c  |  5 +---
 net/sched/sch_fq_codel.c   | 26 -
 net/sched/sch_hhf.c| 33 ++---
 net/sched/sch_netem.c  |  6 +---
 net/sched/sch_sfq.c|  6 +---
 security/keys/keyctl.c | 22 --
 35 files changed, 96 insertions(+), 319 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 4f74511015b8..e6bbb33d2956 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1126,10 +1126,7 @@ static long kvm_s390_get_skeys(struct kvm *kvm, struct 
kvm_s390_skeys *args)
if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
return -EINVAL;
 
-   keys = kmalloc_array(args->count, sizeof(uint8_t),
-GFP_KERNEL | __GFP_NOWARN);
-   if (!keys)
-   keys = vmalloc(sizeof(uint8_t) * args->count);
+   keys = kvmalloc(args->count * sizeof(uint8_t), GFP_KERNEL);
if (!keys)
return -ENOMEM;
 
@@ -1171,10 +1168,7 @@ static long kvm_s390_set_skeys(struct kvm *kvm, struct 
kvm_s390_skeys *args)
if (args->count < 1 || args->count > KVM_S390_SKEYS_MAX)
return -EINVAL;
 
-   keys = kmalloc_array(args->count, sizeof(uint8_t),
-GFP_KERNEL | __GFP_NOWARN);
-   if (!keys)
-   keys = vmalloc(sizeof(uint8_t) * args->count);
+   keys = kvmalloc(sizeof(uint8_t) * args->count, GFP_KERNEL);
if (!keys)
return -ENOMEM;
 
diff --git a/crypto/lzo.c b/crypto/lzo.c
index 168df784da84..218567d717d6 100644
--- a/crypto/lzo.c
+++ b/crypto/lzo.c
@@ -32,9 +32,7 @@ static void *lzo_alloc_ctx(struct