Re: WARNING: kmalloc bug in input_mt_init_slots

Re: WARNING: kmalloc bug in input_mt_init_slots

[Dmitry Vyukov]

On Fri, Sep 21, 2018 at 7:52 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Sep 21, 2018 at 10:24 AM syzbot
> <syzbot+87829a10073277282ad1@syzkaller.appspotmail.com> wrote:
>>
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit:    234b69e3e089 ocfs2: fix ocfs2 read block panic
>> git tree:       upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=131f761a400000
>> kernel config:  https://syzkaller.appspot.com/x/.config?x=5fa12be50bca08d8
>> dashboard link: https://syzkaller.appspot.com/bug?extid=87829a10073277282ad1
>> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=126ca61a400000
>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=119d6511400000
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+87829a10073277282ad1@syzkaller.appspotmail.com
>>
>> input: syz0 as /devices/virtual/input/input25382
>> WARNING: CPU: 0 PID: 11238 at mm/slab_common.c:1031 kmalloc_slab+0x56/0x70
>> mm/slab_common.c:1031
>> Kernel panic - not syncing: panic_on_warn set ...
>
> This is coming from:
>
> commit 6286ae97d10ea2b5cd90532163797ab217bfdbdf
> Author: Christoph Lameter <cl@linux.com>
> Date:   Fri May 3 15:43:18 2013 +0000
>
>    slab: Return NULL for oversized allocations
>
>    The inline path seems to have changed the SLAB behavior for very large
>    kmalloc allocations with  commit e3366016 ("slab: Use common
>    kmalloc_index/kmalloc_size functions"). This patch restores the old
>    behavior but also adds diagnostics so that we can figure where in the
>    code these large allocations occur.
>
>    Reported-and-tested-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>    Signed-off-by: Christoph Lameter <cl@linux.com>
>    Link: http://lkml.kernel.org/r/201305040348.CIF81716.OStQOHFJMFLOVF@I-love.SAKURA.ne.jp
>    [ penberg@kernel.org: use WARN_ON_ONCE ]
>    Signed-off-by: Pekka Enberg <penberg@kernel.org>
>
> You'll have to convince Cristoph that WARN_ON_ONCE() there is evil and
> has to be eradicated so that KASAN can run (but then we'd not know
> easily that some allocation failed because it was too big and never
> had a chance of succeeding vs. ordinary memory failure).
>
> Can I recommend that maybe you introduce infrastructure for
> panic_on_warn to ignore certain "well known" warnings?

Hi Christoph,

What was the motivation behind that WARNING about large allocations in
kmalloc? Why do we want to know about them? Is the general policy that
kmalloc calls with potentially large size requests need to use NOWARN?
If this WARNING still considered useful? Or we should change it to
pr_err?

Re: WARNING: kmalloc bug in input_mt_init_slots

[Christopher Lameter]

On Sun, 23 Sep 2018, Dmitry Vyukov wrote:

> What was the motivation behind that WARNING about large allocations in
> kmalloc? Why do we want to know about them? Is the general policy that
> kmalloc calls with potentially large size requests need to use NOWARN?
> If this WARNING still considered useful? Or we should change it to
> pr_err?

In general large allocs should be satisfied by the page allocator. The
slab allocators are used for allocating and managing small objects. The
page allocator has mechanisms to deal with large objects (compound pages,
multiple page sized allocs etc).

Re: WARNING: kmalloc bug in input_mt_init_slots

[Dmitry Vyukov]

On Mon, Sep 24, 2018 at 5:08 PM, Christopher Lameter <cl@linux.com> wrote:
> On Sun, 23 Sep 2018, Dmitry Vyukov wrote:
>
>> What was the motivation behind that WARNING about large allocations in
>> kmalloc? Why do we want to know about them? Is the general policy that
>> kmalloc calls with potentially large size requests need to use NOWARN?
>> If this WARNING still considered useful? Or we should change it to
>> pr_err?
>
> In general large allocs should be satisfied by the page allocator. The
> slab allocators are used for allocating and managing small objects. The
> page allocator has mechanisms to deal with large objects (compound pages,
> multiple page sized allocs etc).

I am asking more about the status of this warning. If it fires in
input_mt_init_slots(), does it mean that input_mt_init_slots() needs
to be fixed? If not, then we need to change this warning to something
else.

Re: WARNING: kmalloc bug in input_mt_init_slots

[Christopher Lameter]

On Mon, 24 Sep 2018, Dmitry Vyukov wrote:

> On Mon, Sep 24, 2018 at 5:08 PM, Christopher Lameter <cl@linux.com> wrote:
> > On Sun, 23 Sep 2018, Dmitry Vyukov wrote:
> >
> >> What was the motivation behind that WARNING about large allocations in
> >> kmalloc? Why do we want to know about them? Is the general policy that
> >> kmalloc calls with potentially large size requests need to use NOWARN?
> >> If this WARNING still considered useful? Or we should change it to
> >> pr_err?
> >
> > In general large allocs should be satisfied by the page allocator. The
> > slab allocators are used for allocating and managing small objects. The
> > page allocator has mechanisms to deal with large objects (compound pages,
> > multiple page sized allocs etc).
>
> I am asking more about the status of this warning. If it fires in
> input_mt_init_slots(), does it mean that input_mt_init_slots() needs
> to be fixed? If not, then we need to change this warning to something
> else.

Hmmm.. kmalloc falls back to the page allocator already?

See

static __always_inline void *kmalloc(size_t size, gfp_t flags)
{
        if (__builtin_constant_p(size)) {
                if (size > KMALLOC_MAX_CACHE_SIZE)
                        return kmalloc_large(size, flags);


Note that this uses KMALLOC_MAX_CACHE_SIZE which should be smaller than
KMALLOC_MAX_SIZE.


How large is the allocation? AFACIT nRequests larger than KMALLOC_MAX_SIZE
are larger than the maximum allowed by the page allocator. Thus the warning
and the NULL return.

Re: WARNING: kmalloc bug in input_mt_init_slots

[Dmitry Torokhov]

On Mon, Sep 24, 2018 at 03:55:04PM +0000, Christopher Lameter wrote:
> On Mon, 24 Sep 2018, Dmitry Vyukov wrote:
> 
> > On Mon, Sep 24, 2018 at 5:08 PM, Christopher Lameter <cl@linux.com> wrote:
> > > On Sun, 23 Sep 2018, Dmitry Vyukov wrote:
> > >
> > >> What was the motivation behind that WARNING about large allocations in
> > >> kmalloc? Why do we want to know about them? Is the general policy that
> > >> kmalloc calls with potentially large size requests need to use NOWARN?
> > >> If this WARNING still considered useful? Or we should change it to
> > >> pr_err?
> > >
> > > In general large allocs should be satisfied by the page allocator. The
> > > slab allocators are used for allocating and managing small objects. The
> > > page allocator has mechanisms to deal with large objects (compound pages,
> > > multiple page sized allocs etc).
> >
> > I am asking more about the status of this warning. If it fires in
> > input_mt_init_slots(), does it mean that input_mt_init_slots() needs
> > to be fixed? If not, then we need to change this warning to something
> > else.
> 
> Hmmm.. kmalloc falls back to the page allocator already?
> 
> See
> 
> static __always_inline void *kmalloc(size_t size, gfp_t flags)
> {
>         if (__builtin_constant_p(size)) {

It would not be a constant here though.

>                 if (size > KMALLOC_MAX_CACHE_SIZE)
>                         return kmalloc_large(size, flags);
> 
> 
> Note that this uses KMALLOC_MAX_CACHE_SIZE which should be smaller than
> KMALLOC_MAX_SIZE.
> 
> 
> How large is the allocation? AFACIT nRequests larger than KMALLOC_MAX_SIZE
> are larger than the maximum allowed by the page allocator. Thus the warning
> and the NULL return.

The size in this particular case is being derived from a value passed
from userspace. Input core does not care about any limits on size of
memory kmalloc() can support and is perfectly happy with getting NULL
and telling userspace to go away with their silly requests by returning
-ENOMEM.

For the record: I definitely do not want to pre-sanitize size neither in
uinput nor in input core.

Thanks.

-- 
Dmitry

Re: WARNING: kmalloc bug in input_mt_init_slots

[Dmitry Vyukov]

On Mon, Sep 24, 2018 at 8:41 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Sep 24, 2018 at 03:55:04PM +0000, Christopher Lameter wrote:
>> On Mon, 24 Sep 2018, Dmitry Vyukov wrote:
>>
>> > On Mon, Sep 24, 2018 at 5:08 PM, Christopher Lameter <cl@linux.com> wrote:
>> > > On Sun, 23 Sep 2018, Dmitry Vyukov wrote:
>> > >
>> > >> What was the motivation behind that WARNING about large allocations in
>> > >> kmalloc? Why do we want to know about them? Is the general policy that
>> > >> kmalloc calls with potentially large size requests need to use NOWARN?
>> > >> If this WARNING still considered useful? Or we should change it to
>> > >> pr_err?
>> > >
>> > > In general large allocs should be satisfied by the page allocator. The
>> > > slab allocators are used for allocating and managing small objects. The
>> > > page allocator has mechanisms to deal with large objects (compound pages,
>> > > multiple page sized allocs etc).
>> >
>> > I am asking more about the status of this warning. If it fires in
>> > input_mt_init_slots(), does it mean that input_mt_init_slots() needs
>> > to be fixed? If not, then we need to change this warning to something
>> > else.
>>
>> Hmmm.. kmalloc falls back to the page allocator already?
>>
>> See
>>
>> static __always_inline void *kmalloc(size_t size, gfp_t flags)
>> {
>>         if (__builtin_constant_p(size)) {
>
> It would not be a constant here though.
>
>>                 if (size > KMALLOC_MAX_CACHE_SIZE)
>>                         return kmalloc_large(size, flags);
>>
>>
>> Note that this uses KMALLOC_MAX_CACHE_SIZE which should be smaller than
>> KMALLOC_MAX_SIZE.
>>
>>
>> How large is the allocation? AFACIT nRequests larger than KMALLOC_MAX_SIZE
>> are larger than the maximum allowed by the page allocator. Thus the warning
>> and the NULL return.
>
> The size in this particular case is being derived from a value passed
> from userspace. Input core does not care about any limits on size of
> memory kmalloc() can support and is perfectly happy with getting NULL
> and telling userspace to go away with their silly requests by returning
> -ENOMEM.
>
> For the record: I definitely do not want to pre-sanitize size neither in
> uinput nor in input core.

Christopher,

Assuming that the size is large enough to fail in all allocators, is
this warning still useful? How? Should we remove it?

Re: WARNING: kmalloc bug in input_mt_init_slots

[Christopher Lameter]

On Tue, 25 Sep 2018, Dmitry Vyukov wrote:

> Assuming that the size is large enough to fail in all allocators, is
> this warning still useful? How? Should we remove it?

Remove it. It does not make sense because we check earlier if possible
without the warn.

Re: WARNING: kmalloc bug in input_mt_init_slots

[Dmitry Vyukov]

On Tue, Sep 25, 2018 at 4:04 PM, Christopher Lameter <cl@linux.com> wrote:
> On Tue, 25 Sep 2018, Dmitry Vyukov wrote:
>
>> Assuming that the size is large enough to fail in all allocators, is
>> this warning still useful? How? Should we remove it?
>
> Remove it. It does not make sense because we check earlier if possible
> without the warn.

Mailed "mm: don't warn about large allocations for slab" to remove the warning.

Re: WARNING: kmalloc bug in input_mt_init_slots

[Christopher Lameter]

On Thu, 27 Sep 2018, Dmitry Vyukov wrote:

> On Tue, Sep 25, 2018 at 4:04 PM, Christopher Lameter <cl@linux.com> wrote:
> > On Tue, 25 Sep 2018, Dmitry Vyukov wrote:
> >
> >> Assuming that the size is large enough to fail in all allocators, is
> >> this warning still useful? How? Should we remove it?
> >
> > Remove it. It does not make sense because we check earlier if possible
> > without the warn.
>
> Mailed "mm: don't warn about large allocations for slab" to remove the warning.
>

Hoe it arrives here at some point.

Re: WARNING: kmalloc bug in input_mt_init_slots

[Dmitry Vyukov]

On Thu, Sep 27, 2018 at 4:16 PM, Christopher Lameter <cl@linux.com> wrote:
> On Thu, 27 Sep 2018, Dmitry Vyukov wrote:
>
>> On Tue, Sep 25, 2018 at 4:04 PM, Christopher Lameter <cl@linux.com> wrote:
>> > On Tue, 25 Sep 2018, Dmitry Vyukov wrote:
>> >
>> >> Assuming that the size is large enough to fail in all allocators, is
>> >> this warning still useful? How? Should we remove it?
>> >
>> > Remove it. It does not make sense because we check earlier if possible
>> > without the warn.
>>
>> Mailed "mm: don't warn about large allocations for slab" to remove the warning.
>>
>
> Hoe it arrives here at some point.

It's here:
https://lore.kernel.org/patchwork/patch/992660/

Re: WARNING: kmalloc bug in input_mt_init_slots

[Christopher Lameter]

On Thu, 27 Sep 2018, Dmitry Vyukov wrote:

> On Thu, Sep 27, 2018 at 4:16 PM, Christopher Lameter <cl@linux.com> wrote:
> > On Thu, 27 Sep 2018, Dmitry Vyukov wrote:
> >
> >> On Tue, Sep 25, 2018 at 4:04 PM, Christopher Lameter <cl@linux.com> wrote:
> >> > On Tue, 25 Sep 2018, Dmitry Vyukov wrote:
> >> >
> >> >> Assuming that the size is large enough to fail in all allocators, is
> >> >> this warning still useful? How? Should we remove it?
> >> >
> >> > Remove it. It does not make sense because we check earlier if possible
> >> > without the warn.
> >>
> >> Mailed "mm: don't warn about large allocations for slab" to remove the warning.
> >>
> >
> > Hoe it arrives here at some point.
>
> It's here:
> https://lore.kernel.org/patchwork/patch/992660/
>

Please post on the mailing list and NAK to the patch. Testing against
KMALLOC_MAX_CACHE_SIZE is not ok.

Re: WARNING: kmalloc bug in input_mt_init_slots

[Dmitry Vyukov]

On Thu, Sep 27, 2018 at 5:22 PM, Christopher Lameter <cl@linux.com> wrote:
> On Thu, 27 Sep 2018, Dmitry Vyukov wrote:
>
>> On Thu, Sep 27, 2018 at 4:16 PM, Christopher Lameter <cl@linux.com> wrote:
>> > On Thu, 27 Sep 2018, Dmitry Vyukov wrote:
>> >
>> >> On Tue, Sep 25, 2018 at 4:04 PM, Christopher Lameter <cl@linux.com> wrote:
>> >> > On Tue, 25 Sep 2018, Dmitry Vyukov wrote:
>> >> >
>> >> >> Assuming that the size is large enough to fail in all allocators, is
>> >> >> this warning still useful? How? Should we remove it?
>> >> >
>> >> > Remove it. It does not make sense because we check earlier if possible
>> >> > without the warn.
>> >>
>> >> Mailed "mm: don't warn about large allocations for slab" to remove the warning.
>> >>
>> >
>> > Hoe it arrives here at some point.
>>
>> It's here:
>> https://lore.kernel.org/patchwork/patch/992660/
>>
>
> Please post on the mailing list

It is on the  mailing lists:
https://lkml.org/lkml/2018/9/27/802

> and NAK to the patch. Testing against
> KMALLOC_MAX_CACHE_SIZE is not ok.

Why?

Re: WARNING: kmalloc bug in input_mt_init_slots

[Christopher Lameter]

On Thu, 27 Sep 2018, Dmitry Vyukov wrote:

> > Please post on the mailing list
>
> It is on the  mailing lists:
> https://lkml.org/lkml/2018/9/27/802


Ok then lets continue the discussion there.

Re: WARNING: kmalloc bug in input_mt_init_slots

[Matthew Wilcox]

On Mon, Sep 24, 2018 at 11:41:58AM -0700, Dmitry Torokhov wrote:
> > How large is the allocation? AFACIT nRequests larger than KMALLOC_MAX_SIZE
> > are larger than the maximum allowed by the page allocator. Thus the warning
> > and the NULL return.
> 
> The size in this particular case is being derived from a value passed
> from userspace. Input core does not care about any limits on size of
> memory kmalloc() can support and is perfectly happy with getting NULL
> and telling userspace to go away with their silly requests by returning
> -ENOMEM.
> 
> For the record: I definitely do not want to pre-sanitize size neither in
> uinput nor in input core.

Probably should be using kvzalloc then.

Re: WARNING: kmalloc bug in input_mt_init_slots

[Dmitry Torokhov]

On Thu, Sep 27, 2018 at 07:35:37AM -0700, Matthew Wilcox wrote:
> On Mon, Sep 24, 2018 at 11:41:58AM -0700, Dmitry Torokhov wrote:
> > > How large is the allocation? AFACIT nRequests larger than KMALLOC_MAX_SIZE
> > > are larger than the maximum allowed by the page allocator. Thus the warning
> > > and the NULL return.
> > 
> > The size in this particular case is being derived from a value passed
> > from userspace. Input core does not care about any limits on size of
> > memory kmalloc() can support and is perfectly happy with getting NULL
> > and telling userspace to go away with their silly requests by returning
> > -ENOMEM.
> > 
> > For the record: I definitely do not want to pre-sanitize size neither in
> > uinput nor in input core.
> 
> Probably should be using kvzalloc then.

No. No sane input device can track so many contacts so we need to use
kvzalloc(). Failing to allocate memory is proper response here.

Thanks.

-- 
Dmitry

Re: WARNING: kmalloc bug in input_mt_init_slots

[Christopher Lameter]

On Tue, 16 Oct 2018, Dmitry Torokhov wrote:

> On Thu, Sep 27, 2018 at 07:35:37AM -0700, Matthew Wilcox wrote:
> > On Mon, Sep 24, 2018 at 11:41:58AM -0700, Dmitry Torokhov wrote:
> > > > How large is the allocation? AFACIT nRequests larger than KMALLOC_MAX_SIZE
> > > > are larger than the maximum allowed by the page allocator. Thus the warning
> > > > and the NULL return.
> > >
> > > The size in this particular case is being derived from a value passed
> > > from userspace. Input core does not care about any limits on size of
> > > memory kmalloc() can support and is perfectly happy with getting NULL
> > > and telling userspace to go away with their silly requests by returning
> > > -ENOMEM.
> > >
> > > For the record: I definitely do not want to pre-sanitize size neither in
> > > uinput nor in input core.
> >
> > Probably should be using kvzalloc then.
>
> No. No sane input device can track so many contacts so we need to use
> kvzalloc(). Failing to allocate memory is proper response here.

What is a "contact" here? Are we talking about SG segments?

Re: WARNING: kmalloc bug in input_mt_init_slots

[Dmitry Torokhov]

On October 17, 2018 8:35:15 AM PDT, Christopher Lameter <cl@linux.com> wrote:
>On Tue, 16 Oct 2018, Dmitry Torokhov wrote:
>
>> On Thu, Sep 27, 2018 at 07:35:37AM -0700, Matthew Wilcox wrote:
>> > On Mon, Sep 24, 2018 at 11:41:58AM -0700, Dmitry Torokhov wrote:
>> > > > How large is the allocation? AFACIT nRequests larger than
>KMALLOC_MAX_SIZE
>> > > > are larger than the maximum allowed by the page allocator. Thus
>the warning
>> > > > and the NULL return.
>> > >
>> > > The size in this particular case is being derived from a value
>passed
>> > > from userspace. Input core does not care about any limits on size
>of
>> > > memory kmalloc() can support and is perfectly happy with getting
>NULL
>> > > and telling userspace to go away with their silly requests by
>returning
>> > > -ENOMEM.
>> > >
>> > > For the record: I definitely do not want to pre-sanitize size
>neither in
>> > > uinput nor in input core.
>> >
>> > Probably should be using kvzalloc then.
>>
>> No. No sane input device can track so many contacts so we need to use
>> kvzalloc(). Failing to allocate memory is proper response here.
>
>What is a "contact" here? Are we talking about SG segments?

No, we are talking about maximum number of fingers a person can have. Devices don't usually track more than 10 distinct contacts on the touch surface at a time.


Thanks.

-- 
Dmitry

Re: WARNING: kmalloc bug in input_mt_init_slots

[Christopher Lameter]

On Wed, 17 Oct 2018, Dmitry Torokhov wrote:

> >What is a "contact" here? Are we talking about SG segments?
>
> No, we are talking about maximum number of fingers a person can have. Devices don't usually track more than 10 distinct contacts on the touch surface at a time.

Ohh... Way off my usual contexts of development. Sorry.

Ok you have my blessing.