On Wed 11-01-17 20:45:25, Michal Hocko wrote:
> On Wed 11-01-17 09:37:06, Chas Williams wrote:
> > On Mon, 2017-01-09 at 18:20 +0100, Andrey Konovalov wrote:
> > > Hi!
> > >
> > > I've got the following error report while running the syzkaller fuzzer.
> > >
> > > On commit a121103c922847ba5010819a3f250f1f7fc84ab8 (4.10-rc3).
> > >
> > > A reproducer is attached.
> > >
> > > ------------[ cut here ]------------
> > > WARNING: CPU: 0 PID: 4114 at kernel/sched/core.c:7737
> > > __might_sleep+0x149/0x1a0
> > > do not call blocking ops when !TASK_RUNNING; state=1 set at
> > > [<ffffffff813fcb22>] prepare_to_wait+0x182/0x530
> > > Modules linked in:
> > > CPU: 0 PID: 4114 Comm: a.out Not tainted 4.10.0-rc3+ #59
> > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs
> > > 01/01/2011
> > > Call Trace:
> > > __dump_stack lib/dump_stack.c:15
> > > dump_stack+0x292/0x398 lib/dump_stack.c:51
> > > __warn+0x19f/0x1e0 kernel/panic.c:547
> > > warn_slowpath_fmt+0xc5/0x110 kernel/panic.c:562
> > > __might_sleep+0x149/0x1a0 kernel/sched/core.c:7732
> > > slab_pre_alloc_hook mm/slab.h:408
> > > slab_alloc_node mm/slub.c:2634
> > > kmem_cache_alloc_node+0x14a/0x280 mm/slub.c:2744
> > > __alloc_skb+0x10f/0x800 net/core/skbuff.c:219
> > > alloc_skb ./include/linux/skbuff.h:926
> > > alloc_tx net/atm/common.c:75
> >
> > This is likely alloc_skb(..., GFP_KERNEL) in alloc_tx(). The simplest
> > fix for this would be simply to switch this GFP_ATOMIC. See if this is
> > any better.
> >
> > diff --git a/net/atm/common.c b/net/atm/common.c
> > index a3ca922..d84220c 100644
> > --- a/net/atm/common.c
> > +++ b/net/atm/common.c
> > @@ -72,7 +72,7 @@ static struct sk_buff *alloc_tx(struct atm_vcc *vcc,
> > unsigned int size)
> > sk_wmem_alloc_get(sk), size, sk->sk_sndbuf);
> > return NULL;
> > }
> > - while (!(skb = alloc_skb(size, GFP_KERNEL)))
> > + while (!(skb = alloc_skb(size, GFP_ATOMIC)))
> > schedule();
> > pr_debug("%d += %d\n", sk_wmem_alloc_get(sk), skb->truesize);
> > atomic_add(skb->truesize, &sk->sk_wmem_alloc);
>
> Blee, this code is just horrendous. But the "fix" is obviously broken!
> schedule() is just a noop if you do not change the task state and what
> you are just asking for is a never failing non sleeping allocation - aka
> a busy loop in the kernel!
And btw. this while loop should be really turned into GFP_KERNEL |
__GFP_NOFAIL with and explanation why this allocation cannot possibly
fail.
--
Michal Hocko
SUSE Labs
