lkml.org 
[lkml]   [2009]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: packet_sendmsg_spkt sleeping from invalid context
Le 14/12/2009 18:52, Frederic Weisbecker a écrit :
> Hi,
>
> I don't know if it has been reported already.
> I get the following warning on boot, with latest upstream tree:
>
> [ 32.776502] sched: BUG: sleeping function called from invalid context at mm/slab.c:3032
> [ 32.802173] sched: in_atomic(): 1, irqs_disabled(): 0, pid: 3555, name: dhclient3
> [ 32.821141] 1 lock held by dhclient3/3555:
> [ 32.821147] #0: (rcu_read_lock){.+.+.+}, at: [<ffffffff815d177d>] packet_sendmsg_spkt+0x7d/0x2c0
> [ 32.821174] Pid: 3555, comm: dhclient3 Tainted: G W 2.6.32-tip+ #134
> [ 32.821181] Call Trace:
> [ 32.821194] [<ffffffff810811c3>] ? __debug_show_held_locks+0x13/0x30
> [ 32.821207] [<ffffffff8103d6b8>] __might_sleep+0x118/0x140
> [ 32.821219] [<ffffffff81110c23>] kmem_cache_alloc+0x173/0x190
> [ 32.821231] [<ffffffff815483d9>] __alloc_skb+0x49/0x170
> [ 32.821241] [<ffffffff81542238>] sock_wmalloc+0x38/0x80
> [ 32.821250] [<ffffffff815d182b>] packet_sendmsg_spkt+0x12b/0x2c0
> [ 32.821260] [<ffffffff815d177d>] ? packet_sendmsg_spkt+0x7d/0x2c0
> [ 32.821272] [<ffffffff8153ded7>] sock_sendmsg+0x127/0x140
> [ 32.821285] [<ffffffff8106f4a0>] ? autoremove_wake_function+0x0/0x40
> [ 32.821297] [<ffffffff810f467b>] ? might_fault+0x7b/0xd0
> [ 32.821306] [<ffffffff810f467b>] ? might_fault+0x7b/0xd0
> [ 32.821318] [<ffffffff815401aa>] ? move_addr_to_kernel+0x6a/0x70
> [ 32.821328] [<ffffffff8154029f>] sys_sendto+0xef/0x120
> [ 32.821340] [<ffffffff81131479>] ? mntput_no_expire+0x29/0x110
> [ 32.821355] [<ffffffff810027db>] system_call_fastpath+0x16/0x1b
>

Thanks for the report Frederic.

We could partly revert the original commit, but as we wanted to avoid touching
device refcount, and af_packet might be the only real abuser, we could
try following patch instead.

Thanks

[PATCH] packet: dont call sleeping function while holding rcu_read_lock()

commit 654d1f8a019dfa06d (packet: less dev_put() calls)
introduced a problem, calling a potentially sleeping function from a
rcu_read_lock() protected section.

Fix this by releasing lock before the sock_wmalloc() call.
After skb allocation, we redo device lookup and appropriate tests.

Reported-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/packet/af_packet.c | 36 +++++++++++++-----------------------
1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 0205621..19ceadc 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -415,7 +415,7 @@ static int packet_sendmsg_spkt(struct kiocb *iocb, struct socket *sock,
{
struct sock *sk = sock->sk;
struct sockaddr_pkt *saddr = (struct sockaddr_pkt *)msg->msg_name;
- struct sk_buff *skb;
+ struct sk_buff *skb = NULL;
struct net_device *dev;
__be16 proto = 0;
int err;
@@ -437,6 +437,7 @@ static int packet_sendmsg_spkt(struct kiocb *iocb, struct socket *sock,
*/

saddr->spkt_device[13] = 0;
+retry:
rcu_read_lock();
dev = dev_get_by_name_rcu(sock_net(sk), saddr->spkt_device);
err = -ENODEV;
@@ -456,27 +457,21 @@ static int packet_sendmsg_spkt(struct kiocb *iocb, struct socket *sock,
if (len > dev->mtu + dev->hard_header_len)
goto out_unlock;

- err = -ENOBUFS;
- skb = sock_wmalloc(sk, len + LL_RESERVED_SPACE(dev), 0, GFP_KERNEL);
-
- /*
- * If the write buffer is full, then tough. At this level the user
- * gets to deal with the problem - do your own algorithmic backoffs.
- * That's far more flexible.
- */
+ if (!skb) {
+ size_t reserved = LL_RESERVED_SPACE(dev);

- if (skb == NULL)
- goto out_unlock;
-
- /*
- * Fill it in
- */
+ rcu_read_unlock();
+ skb = sock_wmalloc(sk, len + reserved, 0, GFP_KERNEL);
+ if (skb == NULL)
+ return -ENOBUFS;
+ skb_reserve(skb, reserved);
+ goto retry;
+ }

/* FIXME: Save some space for broken drivers that write a
* hard header at transmission time by themselves. PPP is the
* notable one here. This should really be fixed at the driver level.
*/
- skb_reserve(skb, LL_RESERVED_SPACE(dev));
skb_reset_network_header(skb);

/* Try to align data part correctly */
@@ -494,20 +489,15 @@ static int packet_sendmsg_spkt(struct kiocb *iocb, struct socket *sock,
skb->priority = sk->sk_priority;
skb->mark = sk->sk_mark;
if (err)
- goto out_free;
-
- /*
- * Now send it
- */
+ goto out_unlock;

dev_queue_xmit(skb);
rcu_read_unlock();
return len;

-out_free:
- kfree_skb(skb);
out_unlock:
rcu_read_unlock();
+ kfree_skb(skb);
return err;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2009-12-14 20:27    [W:2.960 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site