| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4B2690E7.4030303@gmail.com>
Date: Mon, 14 Dec 2009 20:24:23 +0100
From: Eric Dumazet <eric.dumazet@...il.com>
To: Frederic Weisbecker <fweisbec@...il.com>
CC: "David S. Miller" <davem@...emloft.net>,
Neil Horman <nhorman@...driver.com>,
Netdev <netdev@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: 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@...il.com>
Signed-off-by: Eric Dumazet <eric.dumazet@...il.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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists