Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1554523 > unrolled thread

net/atm: warning in alloc_tx/__might_sleep

Back to article view | Back to linux.kernel

Contents

  net/atm: warning in alloc_tx/__might_sleep Andrey Konovalov <andreyknvl@google.com> - 2017-01-09 18:30 +0100
    Re: net/atm: warning in alloc_tx/__might_sleep Cong Wang <xiyou.wangcong@gmail.com> - 2017-01-10 18:40 +0100
      Re: net/atm: warning in alloc_tx/__might_sleep Eric Dumazet <edumazet@google.com> - 2017-01-10 18:50 +0100
        Re: net/atm: warning in alloc_tx/__might_sleep Cong Wang <xiyou.wangcong@gmail.com> - 2017-01-10 19:00 +0100
        Re: net/atm: warning in alloc_tx/__might_sleep Francois Romieu <romieu@fr.zoreil.com> - 2017-01-10 23:50 +0100
          Re: net/atm: warning in alloc_tx/__might_sleep Eric Dumazet <eric.dumazet@gmail.com> - 2017-01-11 20:20 +0100
        Re: net/atm: warning in alloc_tx/__might_sleep Andrey Konovalov <andreyknvl@google.com> - 2017-01-11 11:20 +0100
    Re: net/atm: warning in alloc_tx/__might_sleep Chas Williams <3chas3@gmail.com> - 2017-01-11 15:40 +0100
      Re: net/atm: warning in alloc_tx/__might_sleep Michal Hocko <mhocko@kernel.org> - 2017-01-11 20:50 +0100
        Re: net/atm: warning in alloc_tx/__might_sleep Cong Wang <xiyou.wangcong@gmail.com> - 2017-01-12 05:40 +0100
          Re: net/atm: warning in alloc_tx/__might_sleep Chas Williams <3chas3@gmail.com> - 2017-01-12 11:50 +0100
      Re: net/atm: warning in alloc_tx/__might_sleep Michal Hocko <mhocko@kernel.org> - 2017-01-11 20:50 +0100

#1554523 — net/atm: warning in alloc_tx/__might_sleep

[Multipart message — attachments visible in raw view] — view raw

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
 vcc_sendmsg+0x5e8/0x1010 net/atm/common.c:609
 sock_sendmsg_nosec net/socket.c:635
 sock_sendmsg+0xca/0x110 net/socket.c:645
 ___sys_sendmsg+0x9d2/0xae0 net/socket.c:1985
 __sys_sendmsg+0x138/0x320 net/socket.c:2019
 SYSC_sendmsg net/socket.c:2030
 SyS_sendmsg+0x2d/0x50 net/socket.c:2026
 entry_SYSCALL_64_fastpath+0x1f/0xc2 arch/x86/entry/entry_64.S:203
RIP: 0033:0x7fcbacfddb79
RSP: 002b:00007ffed8b5a7b8 EFLAGS: 00000206 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007ffed8b5a950 RCX: 00007fcbacfddb79
RDX: 000000000000c000 RSI: 0000000020002000 RDI: 0000000000000003
RBP: 0000000000400af0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000206 R12: 0000000000000000
R13: 00007ffed8b5a950 R14: 0000000000000000 R15: 0000000000000000
---[ end trace 9edf2da84d8112da ]---
atm:sigd_send: bad message type 0

[toc] | [next] | [standalone]

#1555747

[Multipart message — attachments visible in raw view] — view raw

On Mon, Jan 9, 2017 at 9:20 AM, Andrey Konovalov <andreyknvl@google.com> 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
>  vcc_sendmsg+0x5e8/0x1010 net/atm/common.c:609
>  sock_sendmsg_nosec net/socket.c:635
>  sock_sendmsg+0xca/0x110 net/socket.c:645
>  ___sys_sendmsg+0x9d2/0xae0 net/socket.c:1985
>  __sys_sendmsg+0x138/0x320 net/socket.c:2019
>  SYSC_sendmsg net/socket.c:2030
>  SyS_sendmsg+0x2d/0x50 net/socket.c:2026
>  entry_SYSCALL_64_fastpath+0x1f/0xc2 arch/x86/entry/entry_64.S:203
> RIP: 0033:0x7fcbacfddb79
> RSP: 002b:00007ffed8b5a7b8 EFLAGS: 00000206 ORIG_RAX: 000000000000002e
> RAX: ffffffffffffffda RBX: 00007ffed8b5a950 RCX: 00007fcbacfddb79
> RDX: 000000000000c000 RSI: 0000000020002000 RDI: 0000000000000003
> RBP: 0000000000400af0 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000206 R12: 0000000000000000
> R13: 00007ffed8b5a950 R14: 0000000000000000 R15: 0000000000000000
> ---[ end trace 9edf2da84d8112da ]---
> atm:sigd_send: bad message type 0

The fix should be straight-forward. Mind to try the attached patch?

[toc] | [prev] | [next] | [standalone]

#1555761

On Tue, Jan 10, 2017 at 9:35 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Mon, Jan 9, 2017 at 9:20 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
>
> The fix should be straight-forward. Mind to try the attached patch?


You forgot to remove schedule() ?

  schedule();
+ wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);

[toc] | [prev] | [next] | [standalone]

#1555780

On Tue, Jan 10, 2017 at 9:40 AM, Eric Dumazet <edumazet@google.com> wrote:
> On Tue, Jan 10, 2017 at 9:35 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Mon, Jan 9, 2017 at 9:20 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
>>
>> The fix should be straight-forward. Mind to try the attached patch?
>
>
> You forgot to remove schedule() ?
>
>   schedule();
> + wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);

Ah, of course, never even compile it... :-/

[toc] | [prev] | [next] | [standalone]

#1555982

Eric Dumazet <edumazet@google.com> :
> On Tue, Jan 10, 2017 at 9:35 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Mon, Jan 9, 2017 at 9:20 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
> >
> > The fix should be straight-forward. Mind to try the attached patch?
> 
> 
> You forgot to remove schedule() ?

It may be clearer to split alloc_tx in two parts: only the unsleepable
"if (sk_wmem_alloc_get(sk) && !atm_may_send(vcc, size)) {" part of it
contributes to the inner "while (!(skb = alloc_tx(vcc, eff))) {" block.

See net/atm/common.c
[...]
static struct sk_buff *alloc_tx(struct atm_vcc *vcc, unsigned int size)
{
        struct sk_buff *skb;
        struct sock *sk = sk_atm(vcc);

        if (sk_wmem_alloc_get(sk) && !atm_may_send(vcc, size)) {
                pr_debug("Sorry: wmem_alloc = %d, size = %d, sndbuf = %d\n",
                         sk_wmem_alloc_get(sk), size, sk->sk_sndbuf);
                return NULL;
        }
        while (!(skb = alloc_skb(size, GFP_KERNEL)))
                schedule();
        pr_debug("%d += %d\n", sk_wmem_alloc_get(sk), skb->truesize);
        atomic_add(skb->truesize, &sk->sk_wmem_alloc);
        return skb;
}

The waiting stuff is related to vcc drain but the code makes it look as
if it were also related to skb alloc (it isn't).

It may be obvious for you but it took me a while to figure what the
code is supposed to achieve.

-- 
Ueimor

[toc] | [prev] | [next] | [standalone]

#1556855

On Tue, 2017-01-10 at 23:47 +0100, Francois Romieu wrote:
> Eric Dumazet <edumazet@google.com> :
> > On Tue, Jan 10, 2017 at 9:35 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > > On Mon, Jan 9, 2017 at 9:20 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
> > >
> > > The fix should be straight-forward. Mind to try the attached patch?
> > 
> > 
> > You forgot to remove schedule() ?
> 
> It may be clearer to split alloc_tx in two parts: only the unsleepable
> "if (sk_wmem_alloc_get(sk) && !atm_may_send(vcc, size)) {" part of it
> contributes to the inner "while (!(skb = alloc_tx(vcc, eff))) {" block.
> 
> See net/atm/common.c
> [...]
> static struct sk_buff *alloc_tx(struct atm_vcc *vcc, unsigned int size)
> {
>         struct sk_buff *skb;
>         struct sock *sk = sk_atm(vcc);
> 
>         if (sk_wmem_alloc_get(sk) && !atm_may_send(vcc, size)) {
>                 pr_debug("Sorry: wmem_alloc = %d, size = %d, sndbuf = %d\n",
>                          sk_wmem_alloc_get(sk), size, sk->sk_sndbuf);
>                 return NULL;
>         }
>         while (!(skb = alloc_skb(size, GFP_KERNEL)))
>                 schedule();

Yeah, this code looks quite wrong anyway.

We can read it as an infinite loop in some stress conditions or memcg
constraints.


> The waiting stuff is related to vcc drain but the code makes it look as
> if it were also related to skb alloc (it isn't).
> 
> It may be obvious for you but it took me a while to figure what the
> code is supposed to achieve.
> 

[toc] | [prev] | [next] | [standalone]

#1556363

On Tue, Jan 10, 2017 at 6:40 PM, Eric Dumazet <edumazet@google.com> wrote:
> On Tue, Jan 10, 2017 at 9:35 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Mon, Jan 9, 2017 at 9:20 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
>>
>> The fix should be straight-forward. Mind to try the attached patch?

Hi Cong,

Your patch with schedule() removed as suggested by Eric fixes the issue.

Tested-by: Andrey Konovalov <andreyknvl@google.com>

Thanks!

>
>
> You forgot to remove schedule() ?
>
>   schedule();
> + wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);

[toc] | [prev] | [next] | [standalone]

#1556530

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);

[toc] | [prev] | [next] | [standalone]

#1556867

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

[toc] | [prev] | [next] | [standalone]

#1557115

On Wed, Jan 11, 2017 at 11:46 AM, Michal Hocko <mhocko@kernel.org> wrote:
> 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.

I think a nested loop is quite unnecessary, probably due to the code itself
is pretty old. The alloc_tx() is in the outer loop, the alloc_skb() is
in the inner
loop, both seem to wait for a successful GFP allocation. The inner one
is even more unnecessary.

Of course, I am not surprised MM may already have a mechanism to do
the similar logic.

There maybe some reason ATM needs such a logic, although other proto
could handle skb allocation failure quite well in ->sendmsg().

[toc] | [prev] | [next] | [standalone]

#1557347

On Wed, 2017-01-11 at 20:36 -0800, Cong Wang wrote:
> On Wed, Jan 11, 2017 at 11:46 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > 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.
> 
> I think a nested loop is quite unnecessary, probably due to the code itself
> is pretty old. The alloc_tx() is in the outer loop, the alloc_skb() is
> in the inner
> loop, both seem to wait for a successful GFP allocation. The inner one
> is even more unnecessary.
> 
> Of course, I am not surprised MM may already have a mechanism to do
> the similar logic.
> 
> There maybe some reason ATM needs such a logic, although other proto
> could handle skb allocation failure quite well in ->sendmsg().


I can't think of any particular reason that it needs this loop here.  I suspect
that the loop for alloc_tx() predates the wait logic in ->sendmsg() and that the
original looping was in alloc_tx() initially and was simply never removed.  Changes
here would date back to before the git conversion.

[toc] | [prev] | [next] | [standalone]

#1556868

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!

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web