* [PATCH AUTOSEL 4.19 16/37] xfrm interface: fix memory leak on creation
[not found] <20191025135603.25093-1-sashal@kernel.org>
@ 2019-10-25 13:55 ` Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 19/37] net: ipv6: fix listify ip6_rcv_finish in case of forwarding Sasha Levin
` (11 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Nicolas Dichtel, Lorenzo Colitti, Benedict Wong, Steffen Klassert,
Shannon Nelson, Antony Antony, Eyal Birger, Julien Floret,
Sasha Levin, netdev, bpf
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
[ Upstream commit 56c5ee1a5823e9cf5288b84ae6364cb4112f8225 ]
The following commands produce a backtrace and return an error but the xfrm
interface is created (in the wrong netns):
$ ip netns add foo
$ ip netns add bar
$ ip -n foo netns set bar 0
$ ip -n foo link add xfrmi0 link-netnsid 0 type xfrm dev lo if_id 23
RTNETLINK answers: Invalid argument
$ ip -n bar link ls xfrmi0
2: xfrmi0@lo: <NOARP,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
link/none 00:00:00:00:00:00 brd 00:00:00:00:00:00
Here is the backtrace:
[ 79.879174] WARNING: CPU: 0 PID: 1178 at net/core/dev.c:8172 rollback_registered_many+0x86/0x3c1
[ 79.880260] Modules linked in: xfrm_interface nfsv3 nfs_acl auth_rpcgss nfsv4 nfs lockd grace sunrpc fscache button parport_pc parport serio_raw evdev pcspkr loop ext4 crc16 mbcache jbd2 crc32c_generic ide_cd_mod ide_gd_mod cdrom ata_$
eneric ata_piix libata scsi_mod 8139too piix psmouse i2c_piix4 ide_core 8139cp mii i2c_core floppy
[ 79.883698] CPU: 0 PID: 1178 Comm: ip Not tainted 5.2.0-rc6+ #106
[ 79.884462] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
[ 79.885447] RIP: 0010:rollback_registered_many+0x86/0x3c1
[ 79.886120] Code: 01 e8 d7 7d c6 ff 0f 0b 48 8b 45 00 4c 8b 20 48 8d 58 90 49 83 ec 70 48 8d 7b 70 48 39 ef 74 44 8a 83 d0 04 00 00 84 c0 75 1f <0f> 0b e8 61 cd ff ff 48 b8 00 01 00 00 00 00 ad de 48 89 43 70 66
[ 79.888667] RSP: 0018:ffffc900015ab740 EFLAGS: 00010246
[ 79.889339] RAX: ffff8882353e5700 RBX: ffff8882353e56a0 RCX: ffff8882353e5710
[ 79.890174] RDX: ffffc900015ab7e0 RSI: ffffc900015ab7e0 RDI: ffff8882353e5710
[ 79.891029] RBP: ffffc900015ab7e0 R08: ffffc900015ab7e0 R09: ffffc900015ab7e0
[ 79.891866] R10: ffffc900015ab7a0 R11: ffffffff82233fec R12: ffffc900015ab770
[ 79.892728] R13: ffffffff81eb7ec0 R14: ffff88822ed6cf00 R15: 00000000ffffffea
[ 79.893557] FS: 00007ff350f31740(0000) GS:ffff888237a00000(0000) knlGS:0000000000000000
[ 79.894581] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 79.895317] CR2: 00000000006c8580 CR3: 000000022c272000 CR4: 00000000000006f0
[ 79.896137] Call Trace:
[ 79.896464] unregister_netdevice_many+0x12/0x6c
[ 79.896998] __rtnl_newlink+0x6e2/0x73b
[ 79.897446] ? __kmalloc_node_track_caller+0x15e/0x185
[ 79.898039] ? pskb_expand_head+0x5f/0x1fe
[ 79.898556] ? stack_access_ok+0xd/0x2c
[ 79.899009] ? deref_stack_reg+0x12/0x20
[ 79.899462] ? stack_access_ok+0xd/0x2c
[ 79.899927] ? stack_access_ok+0xd/0x2c
[ 79.900404] ? __module_text_address+0x9/0x4f
[ 79.900910] ? is_bpf_text_address+0x5/0xc
[ 79.901390] ? kernel_text_address+0x67/0x7b
[ 79.901884] ? __kernel_text_address+0x1a/0x25
[ 79.902397] ? unwind_get_return_address+0x12/0x23
[ 79.903122] ? __cmpxchg_double_slab.isra.37+0x46/0x77
[ 79.903772] rtnl_newlink+0x43/0x56
[ 79.904217] rtnetlink_rcv_msg+0x200/0x24c
In fact, each time a xfrm interface was created, a netdev was allocated
by __rtnl_newlink()/rtnl_create_link() and then another one by
xfrmi_newlink()/xfrmi_create(). Only the second one was registered, it's
why the previous commands produce a backtrace: dev_change_net_namespace()
was called on a netdev with reg_state set to NETREG_UNINITIALIZED (the
first one).
CC: Lorenzo Colitti <lorenzo@google.com>
CC: Benedict Wong <benedictwong@google.com>
CC: Steffen Klassert <steffen.klassert@secunet.com>
CC: Shannon Nelson <shannon.nelson@oracle.com>
CC: Antony Antony <antony@phenome.org>
CC: Eyal Birger <eyal.birger@gmail.com>
Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
Reported-by: Julien Floret <julien.floret@6wind.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/xfrm/xfrm_interface.c | 98 +++++++++++----------------------------
1 file changed, 28 insertions(+), 70 deletions(-)
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index 555ee2aca6c01..e1265fda30351 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interface.c
@@ -133,7 +133,7 @@ static void xfrmi_dev_free(struct net_device *dev)
free_percpu(dev->tstats);
}
-static int xfrmi_create2(struct net_device *dev)
+static int xfrmi_create(struct net_device *dev)
{
struct xfrm_if *xi = netdev_priv(dev);
struct net *net = dev_net(dev);
@@ -156,54 +156,7 @@ static int xfrmi_create2(struct net_device *dev)
return err;
}
-static struct xfrm_if *xfrmi_create(struct net *net, struct xfrm_if_parms *p)
-{
- struct net_device *dev;
- struct xfrm_if *xi;
- char name[IFNAMSIZ];
- int err;
-
- if (p->name[0]) {
- strlcpy(name, p->name, IFNAMSIZ);
- } else {
- err = -EINVAL;
- goto failed;
- }
-
- dev = alloc_netdev(sizeof(*xi), name, NET_NAME_UNKNOWN, xfrmi_dev_setup);
- if (!dev) {
- err = -EAGAIN;
- goto failed;
- }
-
- dev_net_set(dev, net);
-
- xi = netdev_priv(dev);
- xi->p = *p;
- xi->net = net;
- xi->dev = dev;
- xi->phydev = dev_get_by_index(net, p->link);
- if (!xi->phydev) {
- err = -ENODEV;
- goto failed_free;
- }
-
- err = xfrmi_create2(dev);
- if (err < 0)
- goto failed_dev_put;
-
- return xi;
-
-failed_dev_put:
- dev_put(xi->phydev);
-failed_free:
- free_netdev(dev);
-failed:
- return ERR_PTR(err);
-}
-
-static struct xfrm_if *xfrmi_locate(struct net *net, struct xfrm_if_parms *p,
- int create)
+static struct xfrm_if *xfrmi_locate(struct net *net, struct xfrm_if_parms *p)
{
struct xfrm_if __rcu **xip;
struct xfrm_if *xi;
@@ -211,17 +164,11 @@ static struct xfrm_if *xfrmi_locate(struct net *net, struct xfrm_if_parms *p,
for (xip = &xfrmn->xfrmi[0];
(xi = rtnl_dereference(*xip)) != NULL;
- xip = &xi->next) {
- if (xi->p.if_id == p->if_id) {
- if (create)
- return ERR_PTR(-EEXIST);
-
+ xip = &xi->next)
+ if (xi->p.if_id == p->if_id)
return xi;
- }
- }
- if (!create)
- return ERR_PTR(-ENODEV);
- return xfrmi_create(net, p);
+
+ return NULL;
}
static void xfrmi_dev_uninit(struct net_device *dev)
@@ -689,21 +636,33 @@ static int xfrmi_newlink(struct net *src_net, struct net_device *dev,
struct netlink_ext_ack *extack)
{
struct net *net = dev_net(dev);
- struct xfrm_if_parms *p;
+ struct xfrm_if_parms p;
struct xfrm_if *xi;
+ int err;
- xi = netdev_priv(dev);
- p = &xi->p;
-
- xfrmi_netlink_parms(data, p);
+ xfrmi_netlink_parms(data, &p);
if (!tb[IFLA_IFNAME])
return -EINVAL;
- nla_strlcpy(p->name, tb[IFLA_IFNAME], IFNAMSIZ);
+ nla_strlcpy(p.name, tb[IFLA_IFNAME], IFNAMSIZ);
- xi = xfrmi_locate(net, p, 1);
- return PTR_ERR_OR_ZERO(xi);
+ xi = xfrmi_locate(net, &p);
+ if (xi)
+ return -EEXIST;
+
+ xi = netdev_priv(dev);
+ xi->p = p;
+ xi->net = net;
+ xi->dev = dev;
+ xi->phydev = dev_get_by_index(net, p.link);
+ if (!xi->phydev)
+ return -ENODEV;
+
+ err = xfrmi_create(dev);
+ if (err < 0)
+ dev_put(xi->phydev);
+ return err;
}
static void xfrmi_dellink(struct net_device *dev, struct list_head *head)
@@ -720,9 +679,8 @@ static int xfrmi_changelink(struct net_device *dev, struct nlattr *tb[],
xfrmi_netlink_parms(data, &xi->p);
- xi = xfrmi_locate(net, &xi->p, 0);
-
- if (IS_ERR_OR_NULL(xi)) {
+ xi = xfrmi_locate(net, &xi->p);
+ if (!xi) {
xi = netdev_priv(dev);
} else {
if (xi->dev != dev)
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH AUTOSEL 4.19 19/37] net: ipv6: fix listify ip6_rcv_finish in case of forwarding
[not found] <20191025135603.25093-1-sashal@kernel.org>
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 16/37] xfrm interface: fix memory leak on creation Sasha Levin
@ 2019-10-25 13:55 ` Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 20/37] sch_netem: fix rcu splat in netem_enqueue() Sasha Levin
` (10 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Xin Long, syzbot+eb349eeee854e389c36d,
syzbot+4a0643a653ac375612d1, Edward Cree, David S . Miller,
Sasha Levin, netdev
From: Xin Long <lucien.xin@gmail.com>
[ Upstream commit c7a42eb49212f93a800560662d17d5293960d3c3 ]
We need a similar fix for ipv6 as Commit 0761680d5215 ("net: ipv4: fix
listify ip_rcv_finish in case of forwarding") does for ipv4.
This issue can be reprocuded by syzbot since Commit 323ebb61e32b ("net:
use listified RX for handling GRO_NORMAL skbs") on net-next. The call
trace was:
kernel BUG at include/linux/skbuff.h:2225!
RIP: 0010:__skb_pull include/linux/skbuff.h:2225 [inline]
RIP: 0010:skb_pull+0xea/0x110 net/core/skbuff.c:1902
Call Trace:
sctp_inq_pop+0x2f1/0xd80 net/sctp/inqueue.c:202
sctp_endpoint_bh_rcv+0x184/0x8d0 net/sctp/endpointola.c:385
sctp_inq_push+0x1e4/0x280 net/sctp/inqueue.c:80
sctp_rcv+0x2807/0x3590 net/sctp/input.c:256
sctp6_rcv+0x17/0x30 net/sctp/ipv6.c:1049
ip6_protocol_deliver_rcu+0x2fe/0x1660 net/ipv6/ip6_input.c:397
ip6_input_finish+0x84/0x170 net/ipv6/ip6_input.c:438
NF_HOOK include/linux/netfilter.h:305 [inline]
NF_HOOK include/linux/netfilter.h:299 [inline]
ip6_input+0xe4/0x3f0 net/ipv6/ip6_input.c:447
dst_input include/net/dst.h:442 [inline]
ip6_sublist_rcv_finish+0x98/0x1e0 net/ipv6/ip6_input.c:84
ip6_list_rcv_finish net/ipv6/ip6_input.c:118 [inline]
ip6_sublist_rcv+0x80c/0xcf0 net/ipv6/ip6_input.c:282
ipv6_list_rcv+0x373/0x4b0 net/ipv6/ip6_input.c:316
__netif_receive_skb_list_ptype net/core/dev.c:5049 [inline]
__netif_receive_skb_list_core+0x5fc/0x9d0 net/core/dev.c:5097
__netif_receive_skb_list net/core/dev.c:5149 [inline]
netif_receive_skb_list_internal+0x7eb/0xe60 net/core/dev.c:5244
gro_normal_list.part.0+0x1e/0xb0 net/core/dev.c:5757
gro_normal_list net/core/dev.c:5755 [inline]
gro_normal_one net/core/dev.c:5769 [inline]
napi_frags_finish net/core/dev.c:5782 [inline]
napi_gro_frags+0xa6a/0xea0 net/core/dev.c:5855
tun_get_user+0x2e98/0x3fa0 drivers/net/tun.c:1974
tun_chr_write_iter+0xbd/0x156 drivers/net/tun.c:2020
Fixes: d8269e2cbf90 ("net: ipv6: listify ipv6_rcv() and ip6_rcv_finish()")
Fixes: 323ebb61e32b ("net: use listified RX for handling GRO_NORMAL skbs")
Reported-by: syzbot+eb349eeee854e389c36d@syzkaller.appspotmail.com
Reported-by: syzbot+4a0643a653ac375612d1@syzkaller.appspotmail.com
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Edward Cree <ecree@solarflare.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/ipv6/ip6_input.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 2b6d430223837..acf0749ee5bbd 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -80,8 +80,10 @@ static void ip6_sublist_rcv_finish(struct list_head *head)
{
struct sk_buff *skb, *next;
- list_for_each_entry_safe(skb, next, head, list)
+ list_for_each_entry_safe(skb, next, head, list) {
+ skb_list_del_init(skb);
dst_input(skb);
+ }
}
static void ip6_list_rcv_finish(struct net *net, struct sock *sk,
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH AUTOSEL 4.19 20/37] sch_netem: fix rcu splat in netem_enqueue()
[not found] <20191025135603.25093-1-sashal@kernel.org>
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 16/37] xfrm interface: fix memory leak on creation Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 19/37] net: ipv6: fix listify ip6_rcv_finish in case of forwarding Sasha Levin
@ 2019-10-25 13:55 ` Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 21/37] rxrpc: Fix call ref leak Sasha Levin
` (9 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Eric Dumazet, syzbot, David S . Miller, Sasha Levin, netdev
From: Eric Dumazet <edumazet@google.com>
[ Upstream commit 159d2c7d8106177bd9a986fd005a311fe0d11285 ]
qdisc_root() use from netem_enqueue() triggers a lockdep warning.
__dev_queue_xmit() uses rcu_read_lock_bh() which is
not equivalent to rcu_read_lock() + local_bh_disable_bh as far
as lockdep is concerned.
WARNING: suspicious RCU usage
5.3.0-rc7+ #0 Not tainted
-----------------------------
include/net/sch_generic.h:492 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
3 locks held by syz-executor427/8855:
#0: 00000000b5525c01 (rcu_read_lock_bh){....}, at: lwtunnel_xmit_redirect include/net/lwtunnel.h:92 [inline]
#0: 00000000b5525c01 (rcu_read_lock_bh){....}, at: ip_finish_output2+0x2dc/0x2570 net/ipv4/ip_output.c:214
#1: 00000000b5525c01 (rcu_read_lock_bh){....}, at: __dev_queue_xmit+0x20a/0x3650 net/core/dev.c:3804
#2: 00000000364bae92 (&(&sch->q.lock)->rlock){+.-.}, at: spin_lock include/linux/spinlock.h:338 [inline]
#2: 00000000364bae92 (&(&sch->q.lock)->rlock){+.-.}, at: __dev_xmit_skb net/core/dev.c:3502 [inline]
#2: 00000000364bae92 (&(&sch->q.lock)->rlock){+.-.}, at: __dev_queue_xmit+0x14b8/0x3650 net/core/dev.c:3838
stack backtrace:
CPU: 0 PID: 8855 Comm: syz-executor427 Not tainted 5.3.0-rc7+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
lockdep_rcu_suspicious+0x153/0x15d kernel/locking/lockdep.c:5357
qdisc_root include/net/sch_generic.h:492 [inline]
netem_enqueue+0x1cfb/0x2d80 net/sched/sch_netem.c:479
__dev_xmit_skb net/core/dev.c:3527 [inline]
__dev_queue_xmit+0x15d2/0x3650 net/core/dev.c:3838
dev_queue_xmit+0x18/0x20 net/core/dev.c:3902
neigh_hh_output include/net/neighbour.h:500 [inline]
neigh_output include/net/neighbour.h:509 [inline]
ip_finish_output2+0x1726/0x2570 net/ipv4/ip_output.c:228
__ip_finish_output net/ipv4/ip_output.c:308 [inline]
__ip_finish_output+0x5fc/0xb90 net/ipv4/ip_output.c:290
ip_finish_output+0x38/0x1f0 net/ipv4/ip_output.c:318
NF_HOOK_COND include/linux/netfilter.h:294 [inline]
ip_mc_output+0x292/0xf40 net/ipv4/ip_output.c:417
dst_output include/net/dst.h:436 [inline]
ip_local_out+0xbb/0x190 net/ipv4/ip_output.c:125
ip_send_skb+0x42/0xf0 net/ipv4/ip_output.c:1555
udp_send_skb.isra.0+0x6b2/0x1160 net/ipv4/udp.c:887
udp_sendmsg+0x1e96/0x2820 net/ipv4/udp.c:1174
inet_sendmsg+0x9e/0xe0 net/ipv4/af_inet.c:807
sock_sendmsg_nosec net/socket.c:637 [inline]
sock_sendmsg+0xd7/0x130 net/socket.c:657
___sys_sendmsg+0x3e2/0x920 net/socket.c:2311
__sys_sendmmsg+0x1bf/0x4d0 net/socket.c:2413
__do_sys_sendmmsg net/socket.c:2442 [inline]
__se_sys_sendmmsg net/socket.c:2439 [inline]
__x64_sys_sendmmsg+0x9d/0x100 net/socket.c:2439
do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:296
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
include/net/sch_generic.h | 5 +++++
net/sched/sch_netem.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index c44da48de7dfd..c9cd5086bd544 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -421,6 +421,11 @@ static inline struct Qdisc *qdisc_root(const struct Qdisc *qdisc)
return q;
}
+static inline struct Qdisc *qdisc_root_bh(const struct Qdisc *qdisc)
+{
+ return rcu_dereference_bh(qdisc->dev_queue->qdisc);
+}
+
static inline struct Qdisc *qdisc_root_sleeping(const struct Qdisc *qdisc)
{
return qdisc->dev_queue->qdisc_sleeping;
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 86350fe5cfc8f..15f8f24c190d4 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -474,7 +474,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
* skb will be queued.
*/
if (count > 1 && (skb2 = skb_clone(skb, GFP_ATOMIC)) != NULL) {
- struct Qdisc *rootq = qdisc_root(sch);
+ struct Qdisc *rootq = qdisc_root_bh(sch);
u32 dupsave = q->duplicate; /* prevent duplicating a dup... */
q->duplicate = 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH AUTOSEL 4.19 21/37] rxrpc: Fix call ref leak
[not found] <20191025135603.25093-1-sashal@kernel.org>
` (2 preceding siblings ...)
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 20/37] sch_netem: fix rcu splat in netem_enqueue() Sasha Levin
@ 2019-10-25 13:55 ` Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 22/37] rxrpc: Fix trace-after-put looking at the put peer record Sasha Levin
` (8 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: David Howells, syzbot+d850c266e3df14da1d31, Sasha Levin,
linux-afs, netdev
From: David Howells <dhowells@redhat.com>
[ Upstream commit c48fc11b69e95007109206311b0187a3090591f3 ]
When sendmsg() finds a call to continue on with, if the call is in an
inappropriate state, it doesn't release the ref it just got on that call
before returning an error.
This causes the following symptom to show up with kasan:
BUG: KASAN: use-after-free in rxrpc_send_keepalive+0x8a2/0x940
net/rxrpc/output.c:635
Read of size 8 at addr ffff888064219698 by task kworker/0:3/11077
where line 635 is:
whdr.epoch = htonl(peer->local->rxnet->epoch);
The local endpoint (which cannot be pinned by the call) has been released,
but not the peer (which is pinned by the call).
Fix this by releasing the call in the error path.
Fixes: 37411cad633f ("rxrpc: Fix potential NULL-pointer exception")
Reported-by: syzbot+d850c266e3df14da1d31@syzkaller.appspotmail.com
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/rxrpc/sendmsg.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index 5d6ab4f6fd7ab..3e54ead1e921a 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -661,6 +661,7 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
case RXRPC_CALL_SERVER_PREALLOC:
case RXRPC_CALL_SERVER_SECURING:
case RXRPC_CALL_SERVER_ACCEPTING:
+ rxrpc_put_call(call, rxrpc_call_put);
ret = -EBUSY;
goto error_release_sock;
default:
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH AUTOSEL 4.19 22/37] rxrpc: Fix trace-after-put looking at the put peer record
[not found] <20191025135603.25093-1-sashal@kernel.org>
` (3 preceding siblings ...)
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 21/37] rxrpc: Fix call ref leak Sasha Levin
@ 2019-10-25 13:55 ` Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 23/37] rxrpc: rxrpc_peer needs to hold a ref on the rxrpc_local record Sasha Levin
` (7 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: David Howells, syzbot+b9be979c55f2bea8ed30, Sasha Levin,
linux-afs, netdev
From: David Howells <dhowells@redhat.com>
[ Upstream commit 55f6c98e3674ce16038a1949c3f9ca5a9a99f289 ]
rxrpc_put_peer() calls trace_rxrpc_peer() after it has done the decrement
of the refcount - which looks at the debug_id in the peer record. But
unless the refcount was reduced to zero, we no longer have the right to
look in the record and, indeed, it may be deleted by some other thread.
Fix this by getting the debug_id out before decrementing the refcount and
then passing that into the tracepoint.
This can cause the following symptoms:
BUG: KASAN: use-after-free in __rxrpc_put_peer net/rxrpc/peer_object.c:411
[inline]
BUG: KASAN: use-after-free in rxrpc_put_peer+0x685/0x6a0
net/rxrpc/peer_object.c:435
Read of size 8 at addr ffff888097ec0058 by task syz-executor823/24216
Fixes: 1159d4b496f5 ("rxrpc: Add a tracepoint to track rxrpc_peer refcounting")
Reported-by: syzbot+b9be979c55f2bea8ed30@syzkaller.appspotmail.com
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
include/trace/events/rxrpc.h | 6 +++---
net/rxrpc/peer_object.c | 11 +++++++----
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 0fe169c6afd84..a08916eb76152 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -527,10 +527,10 @@ TRACE_EVENT(rxrpc_local,
);
TRACE_EVENT(rxrpc_peer,
- TP_PROTO(struct rxrpc_peer *peer, enum rxrpc_peer_trace op,
+ TP_PROTO(unsigned int peer_debug_id, enum rxrpc_peer_trace op,
int usage, const void *where),
- TP_ARGS(peer, op, usage, where),
+ TP_ARGS(peer_debug_id, op, usage, where),
TP_STRUCT__entry(
__field(unsigned int, peer )
@@ -540,7 +540,7 @@ TRACE_EVENT(rxrpc_peer,
),
TP_fast_assign(
- __entry->peer = peer->debug_id;
+ __entry->peer = peer_debug_id;
__entry->op = op;
__entry->usage = usage;
__entry->where = where;
diff --git a/net/rxrpc/peer_object.c b/net/rxrpc/peer_object.c
index 71547e8673b99..72b4ad210426e 100644
--- a/net/rxrpc/peer_object.c
+++ b/net/rxrpc/peer_object.c
@@ -386,7 +386,7 @@ struct rxrpc_peer *rxrpc_get_peer(struct rxrpc_peer *peer)
int n;
n = atomic_inc_return(&peer->usage);
- trace_rxrpc_peer(peer, rxrpc_peer_got, n, here);
+ trace_rxrpc_peer(peer->debug_id, rxrpc_peer_got, n, here);
return peer;
}
@@ -400,7 +400,7 @@ struct rxrpc_peer *rxrpc_get_peer_maybe(struct rxrpc_peer *peer)
if (peer) {
int n = atomic_fetch_add_unless(&peer->usage, 1, 0);
if (n > 0)
- trace_rxrpc_peer(peer, rxrpc_peer_got, n + 1, here);
+ trace_rxrpc_peer(peer->debug_id, rxrpc_peer_got, n + 1, here);
else
peer = NULL;
}
@@ -430,11 +430,13 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer)
void rxrpc_put_peer(struct rxrpc_peer *peer)
{
const void *here = __builtin_return_address(0);
+ unsigned int debug_id;
int n;
if (peer) {
+ debug_id = peer->debug_id;
n = atomic_dec_return(&peer->usage);
- trace_rxrpc_peer(peer, rxrpc_peer_put, n, here);
+ trace_rxrpc_peer(debug_id, rxrpc_peer_put, n, here);
if (n == 0)
__rxrpc_put_peer(peer);
}
@@ -447,10 +449,11 @@ void rxrpc_put_peer(struct rxrpc_peer *peer)
void rxrpc_put_peer_locked(struct rxrpc_peer *peer)
{
const void *here = __builtin_return_address(0);
+ unsigned int debug_id = peer->debug_id;
int n;
n = atomic_dec_return(&peer->usage);
- trace_rxrpc_peer(peer, rxrpc_peer_put, n, here);
+ trace_rxrpc_peer(debug_id, rxrpc_peer_put, n, here);
if (n == 0) {
hash_del_rcu(&peer->hash_link);
list_del_init(&peer->keepalive_link);
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH AUTOSEL 4.19 23/37] rxrpc: rxrpc_peer needs to hold a ref on the rxrpc_local record
[not found] <20191025135603.25093-1-sashal@kernel.org>
` (4 preceding siblings ...)
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 22/37] rxrpc: Fix trace-after-put looking at the put peer record Sasha Levin
@ 2019-10-25 13:55 ` Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 24/37] llc: fix sk_buff leak in llc_sap_state_process() Sasha Levin
` (6 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: David Howells, syzbot+b9be979c55f2bea8ed30, Sasha Levin,
linux-afs, netdev
From: David Howells <dhowells@redhat.com>
[ Upstream commit 9ebeddef58c41bd700419cdcece24cf64ce32276 ]
The rxrpc_peer record needs to hold a reference on the rxrpc_local record
it points as the peer is used as a base to access information in the
rxrpc_local record.
This can cause problems in __rxrpc_put_peer(), where we need the network
namespace pointer, and in rxrpc_send_keepalive(), where we need to access
the UDP socket, leading to symptoms like:
BUG: KASAN: use-after-free in __rxrpc_put_peer net/rxrpc/peer_object.c:411
[inline]
BUG: KASAN: use-after-free in rxrpc_put_peer+0x685/0x6a0
net/rxrpc/peer_object.c:435
Read of size 8 at addr ffff888097ec0058 by task syz-executor823/24216
Fix this by taking a ref on the local record for the peer record.
Fixes: ace45bec6d77 ("rxrpc: Fix firewall route keepalive")
Fixes: 2baec2c3f854 ("rxrpc: Support network namespacing")
Reported-by: syzbot+b9be979c55f2bea8ed30@syzkaller.appspotmail.com
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/rxrpc/peer_object.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/rxrpc/peer_object.c b/net/rxrpc/peer_object.c
index 72b4ad210426e..b91b090217cdb 100644
--- a/net/rxrpc/peer_object.c
+++ b/net/rxrpc/peer_object.c
@@ -220,7 +220,7 @@ struct rxrpc_peer *rxrpc_alloc_peer(struct rxrpc_local *local, gfp_t gfp)
peer = kzalloc(sizeof(struct rxrpc_peer), gfp);
if (peer) {
atomic_set(&peer->usage, 1);
- peer->local = local;
+ peer->local = rxrpc_get_local(local);
INIT_HLIST_HEAD(&peer->error_targets);
peer->service_conns = RB_ROOT;
seqlock_init(&peer->service_conn_lock);
@@ -311,7 +311,6 @@ void rxrpc_new_incoming_peer(struct rxrpc_sock *rx, struct rxrpc_local *local,
unsigned long hash_key;
hash_key = rxrpc_peer_hash_key(local, &peer->srx);
- peer->local = local;
rxrpc_init_peer(rx, peer, hash_key);
spin_lock(&rxnet->peer_hash_lock);
@@ -421,6 +420,7 @@ static void __rxrpc_put_peer(struct rxrpc_peer *peer)
list_del_init(&peer->keepalive_link);
spin_unlock_bh(&rxnet->peer_hash_lock);
+ rxrpc_put_local(peer->local);
kfree_rcu(peer, rcu);
}
@@ -457,6 +457,7 @@ void rxrpc_put_peer_locked(struct rxrpc_peer *peer)
if (n == 0) {
hash_del_rcu(&peer->hash_link);
list_del_init(&peer->keepalive_link);
+ rxrpc_put_local(peer->local);
kfree_rcu(peer, rcu);
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH AUTOSEL 4.19 24/37] llc: fix sk_buff leak in llc_sap_state_process()
[not found] <20191025135603.25093-1-sashal@kernel.org>
` (5 preceding siblings ...)
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 23/37] rxrpc: rxrpc_peer needs to hold a ref on the rxrpc_local record Sasha Levin
@ 2019-10-25 13:55 ` Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 25/37] llc: fix sk_buff leak in llc_conn_service() Sasha Levin
` (5 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Eric Biggers, syzbot+6bf095f9becf5efef645,
syzbot+31c16aa4202dace3812e, Jakub Kicinski, Sasha Levin, netdev
From: Eric Biggers <ebiggers@google.com>
[ Upstream commit c6ee11c39fcc1fb55130748990a8f199e76263b4 ]
syzbot reported:
BUG: memory leak
unreferenced object 0xffff888116270800 (size 224):
comm "syz-executor641", pid 7047, jiffies 4294947360 (age 13.860s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 20 e1 2a 81 88 ff ff 00 40 3d 2a 81 88 ff ff . .*.....@=*....
backtrace:
[<000000004d41b4cc>] kmemleak_alloc_recursive include/linux/kmemleak.h:55 [inline]
[<000000004d41b4cc>] slab_post_alloc_hook mm/slab.h:439 [inline]
[<000000004d41b4cc>] slab_alloc_node mm/slab.c:3269 [inline]
[<000000004d41b4cc>] kmem_cache_alloc_node+0x153/0x2a0 mm/slab.c:3579
[<00000000506a5965>] __alloc_skb+0x6e/0x210 net/core/skbuff.c:198
[<000000001ba5a161>] alloc_skb include/linux/skbuff.h:1058 [inline]
[<000000001ba5a161>] alloc_skb_with_frags+0x5f/0x250 net/core/skbuff.c:5327
[<0000000047d9c78b>] sock_alloc_send_pskb+0x269/0x2a0 net/core/sock.c:2225
[<000000003828fe54>] sock_alloc_send_skb+0x32/0x40 net/core/sock.c:2242
[<00000000e34d94f9>] llc_ui_sendmsg+0x10a/0x540 net/llc/af_llc.c:933
[<00000000de2de3fb>] sock_sendmsg_nosec net/socket.c:652 [inline]
[<00000000de2de3fb>] sock_sendmsg+0x54/0x70 net/socket.c:671
[<000000008fe16e7a>] __sys_sendto+0x148/0x1f0 net/socket.c:1964
[...]
The bug is that llc_sap_state_process() always takes an extra reference
to the skb, but sometimes neither llc_sap_next_state() nor
llc_sap_state_process() itself drops this reference.
Fix it by changing llc_sap_next_state() to never consume a reference to
the skb, rather than sometimes do so and sometimes not. Then remove the
extra skb_get() and kfree_skb() from llc_sap_state_process().
Reported-by: syzbot+6bf095f9becf5efef645@syzkaller.appspotmail.com
Reported-by: syzbot+31c16aa4202dace3812e@syzkaller.appspotmail.com
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/llc/llc_s_ac.c | 12 +++++++++---
net/llc/llc_sap.c | 23 ++++++++---------------
2 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/net/llc/llc_s_ac.c b/net/llc/llc_s_ac.c
index a94bd56bcac6f..7ae4cc684d3ab 100644
--- a/net/llc/llc_s_ac.c
+++ b/net/llc/llc_s_ac.c
@@ -58,8 +58,10 @@ int llc_sap_action_send_ui(struct llc_sap *sap, struct sk_buff *skb)
ev->daddr.lsap, LLC_PDU_CMD);
llc_pdu_init_as_ui_cmd(skb);
rc = llc_mac_hdr_init(skb, ev->saddr.mac, ev->daddr.mac);
- if (likely(!rc))
+ if (likely(!rc)) {
+ skb_get(skb);
rc = dev_queue_xmit(skb);
+ }
return rc;
}
@@ -81,8 +83,10 @@ int llc_sap_action_send_xid_c(struct llc_sap *sap, struct sk_buff *skb)
ev->daddr.lsap, LLC_PDU_CMD);
llc_pdu_init_as_xid_cmd(skb, LLC_XID_NULL_CLASS_2, 0);
rc = llc_mac_hdr_init(skb, ev->saddr.mac, ev->daddr.mac);
- if (likely(!rc))
+ if (likely(!rc)) {
+ skb_get(skb);
rc = dev_queue_xmit(skb);
+ }
return rc;
}
@@ -135,8 +139,10 @@ int llc_sap_action_send_test_c(struct llc_sap *sap, struct sk_buff *skb)
ev->daddr.lsap, LLC_PDU_CMD);
llc_pdu_init_as_test_cmd(skb);
rc = llc_mac_hdr_init(skb, ev->saddr.mac, ev->daddr.mac);
- if (likely(!rc))
+ if (likely(!rc)) {
+ skb_get(skb);
rc = dev_queue_xmit(skb);
+ }
return rc;
}
diff --git a/net/llc/llc_sap.c b/net/llc/llc_sap.c
index a7f7b8ff47292..be419062e19a6 100644
--- a/net/llc/llc_sap.c
+++ b/net/llc/llc_sap.c
@@ -197,29 +197,22 @@ static int llc_sap_next_state(struct llc_sap *sap, struct sk_buff *skb)
* After executing actions of the event, upper layer will be indicated
* if needed(on receiving an UI frame). sk can be null for the
* datalink_proto case.
+ *
+ * This function always consumes a reference to the skb.
*/
static void llc_sap_state_process(struct llc_sap *sap, struct sk_buff *skb)
{
struct llc_sap_state_ev *ev = llc_sap_ev(skb);
- /*
- * We have to hold the skb, because llc_sap_next_state
- * will kfree it in the sending path and we need to
- * look at the skb->cb, where we encode llc_sap_state_ev.
- */
- skb_get(skb);
ev->ind_cfm_flag = 0;
llc_sap_next_state(sap, skb);
- if (ev->ind_cfm_flag == LLC_IND) {
- if (skb->sk->sk_state == TCP_LISTEN)
- kfree_skb(skb);
- else {
- llc_save_primitive(skb->sk, skb, ev->prim);
- /* queue skb to the user. */
- if (sock_queue_rcv_skb(skb->sk, skb))
- kfree_skb(skb);
- }
+ if (ev->ind_cfm_flag == LLC_IND && skb->sk->sk_state != TCP_LISTEN) {
+ llc_save_primitive(skb->sk, skb, ev->prim);
+
+ /* queue skb to the user. */
+ if (sock_queue_rcv_skb(skb->sk, skb) == 0)
+ return;
}
kfree_skb(skb);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH AUTOSEL 4.19 25/37] llc: fix sk_buff leak in llc_conn_service()
[not found] <20191025135603.25093-1-sashal@kernel.org>
` (6 preceding siblings ...)
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 24/37] llc: fix sk_buff leak in llc_sap_state_process() Sasha Levin
@ 2019-10-25 13:55 ` Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 26/37] NFC: pn533: fix use-after-free and memleaks Sasha Levin
` (4 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Eric Biggers, syzbot+6b825a6494a04cc0e3f7, Jakub Kicinski,
Sasha Levin, netdev
From: Eric Biggers <ebiggers@google.com>
[ Upstream commit b74555de21acd791f12c4a1aeaf653dd7ac21133 ]
syzbot reported:
BUG: memory leak
unreferenced object 0xffff88811eb3de00 (size 224):
comm "syz-executor559", pid 7315, jiffies 4294943019 (age 10.300s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 a0 38 24 81 88 ff ff 00 c0 f2 15 81 88 ff ff ..8$............
backtrace:
[<000000008d1c66a1>] kmemleak_alloc_recursive include/linux/kmemleak.h:55 [inline]
[<000000008d1c66a1>] slab_post_alloc_hook mm/slab.h:439 [inline]
[<000000008d1c66a1>] slab_alloc_node mm/slab.c:3269 [inline]
[<000000008d1c66a1>] kmem_cache_alloc_node+0x153/0x2a0 mm/slab.c:3579
[<00000000447d9496>] __alloc_skb+0x6e/0x210 net/core/skbuff.c:198
[<000000000cdbf82f>] alloc_skb include/linux/skbuff.h:1058 [inline]
[<000000000cdbf82f>] llc_alloc_frame+0x66/0x110 net/llc/llc_sap.c:54
[<000000002418b52e>] llc_conn_ac_send_sabme_cmd_p_set_x+0x2f/0x140 net/llc/llc_c_ac.c:777
[<000000001372ae17>] llc_exec_conn_trans_actions net/llc/llc_conn.c:475 [inline]
[<000000001372ae17>] llc_conn_service net/llc/llc_conn.c:400 [inline]
[<000000001372ae17>] llc_conn_state_process+0x1ac/0x640 net/llc/llc_conn.c:75
[<00000000f27e53c1>] llc_establish_connection+0x110/0x170 net/llc/llc_if.c:109
[<00000000291b2ca0>] llc_ui_connect+0x10e/0x370 net/llc/af_llc.c:477
[<000000000f9c740b>] __sys_connect+0x11d/0x170 net/socket.c:1840
[...]
The bug is that most callers of llc_conn_send_pdu() assume it consumes a
reference to the skb, when actually due to commit b85ab56c3f81 ("llc:
properly handle dev_queue_xmit() return value") it doesn't.
Revert most of that commit, and instead make the few places that need
llc_conn_send_pdu() to *not* consume a reference call skb_get() before.
Fixes: b85ab56c3f81 ("llc: properly handle dev_queue_xmit() return value")
Reported-by: syzbot+6b825a6494a04cc0e3f7@syzkaller.appspotmail.com
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
include/net/llc_conn.h | 2 +-
net/llc/llc_c_ac.c | 8 ++++++--
net/llc/llc_conn.c | 32 +++++++++-----------------------
3 files changed, 16 insertions(+), 26 deletions(-)
diff --git a/include/net/llc_conn.h b/include/net/llc_conn.h
index df528a6235487..ea985aa7a6c5e 100644
--- a/include/net/llc_conn.h
+++ b/include/net/llc_conn.h
@@ -104,7 +104,7 @@ void llc_sk_reset(struct sock *sk);
/* Access to a connection */
int llc_conn_state_process(struct sock *sk, struct sk_buff *skb);
-int llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb);
+void llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb);
void llc_conn_rtn_pdu(struct sock *sk, struct sk_buff *skb);
void llc_conn_resend_i_pdu_as_cmd(struct sock *sk, u8 nr, u8 first_p_bit);
void llc_conn_resend_i_pdu_as_rsp(struct sock *sk, u8 nr, u8 first_f_bit);
diff --git a/net/llc/llc_c_ac.c b/net/llc/llc_c_ac.c
index 4d78375f9872d..647c0554d04cd 100644
--- a/net/llc/llc_c_ac.c
+++ b/net/llc/llc_c_ac.c
@@ -372,6 +372,7 @@ int llc_conn_ac_send_i_cmd_p_set_1(struct sock *sk, struct sk_buff *skb)
llc_pdu_init_as_i_cmd(skb, 1, llc->vS, llc->vR);
rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
if (likely(!rc)) {
+ skb_get(skb);
llc_conn_send_pdu(sk, skb);
llc_conn_ac_inc_vs_by_1(sk, skb);
}
@@ -389,7 +390,8 @@ static int llc_conn_ac_send_i_cmd_p_set_0(struct sock *sk, struct sk_buff *skb)
llc_pdu_init_as_i_cmd(skb, 0, llc->vS, llc->vR);
rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
if (likely(!rc)) {
- rc = llc_conn_send_pdu(sk, skb);
+ skb_get(skb);
+ llc_conn_send_pdu(sk, skb);
llc_conn_ac_inc_vs_by_1(sk, skb);
}
return rc;
@@ -406,6 +408,7 @@ int llc_conn_ac_send_i_xxx_x_set_0(struct sock *sk, struct sk_buff *skb)
llc_pdu_init_as_i_cmd(skb, 0, llc->vS, llc->vR);
rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
if (likely(!rc)) {
+ skb_get(skb);
llc_conn_send_pdu(sk, skb);
llc_conn_ac_inc_vs_by_1(sk, skb);
}
@@ -916,7 +919,8 @@ static int llc_conn_ac_send_i_rsp_f_set_ackpf(struct sock *sk,
llc_pdu_init_as_i_cmd(skb, llc->ack_pf, llc->vS, llc->vR);
rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
if (likely(!rc)) {
- rc = llc_conn_send_pdu(sk, skb);
+ skb_get(skb);
+ llc_conn_send_pdu(sk, skb);
llc_conn_ac_inc_vs_by_1(sk, skb);
}
return rc;
diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index 4ff89cb7c86f7..ed2aca12460ca 100644
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -30,7 +30,7 @@
#endif
static int llc_find_offset(int state, int ev_type);
-static int llc_conn_send_pdus(struct sock *sk, struct sk_buff *skb);
+static void llc_conn_send_pdus(struct sock *sk);
static int llc_conn_service(struct sock *sk, struct sk_buff *skb);
static int llc_exec_conn_trans_actions(struct sock *sk,
struct llc_conn_state_trans *trans,
@@ -193,11 +193,11 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
return rc;
}
-int llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb)
+void llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb)
{
/* queue PDU to send to MAC layer */
skb_queue_tail(&sk->sk_write_queue, skb);
- return llc_conn_send_pdus(sk, skb);
+ llc_conn_send_pdus(sk);
}
/**
@@ -255,7 +255,7 @@ void llc_conn_resend_i_pdu_as_cmd(struct sock *sk, u8 nr, u8 first_p_bit)
if (howmany_resend > 0)
llc->vS = (llc->vS + 1) % LLC_2_SEQ_NBR_MODULO;
/* any PDUs to re-send are queued up; start sending to MAC */
- llc_conn_send_pdus(sk, NULL);
+ llc_conn_send_pdus(sk);
out:;
}
@@ -296,7 +296,7 @@ void llc_conn_resend_i_pdu_as_rsp(struct sock *sk, u8 nr, u8 first_f_bit)
if (howmany_resend > 0)
llc->vS = (llc->vS + 1) % LLC_2_SEQ_NBR_MODULO;
/* any PDUs to re-send are queued up; start sending to MAC */
- llc_conn_send_pdus(sk, NULL);
+ llc_conn_send_pdus(sk);
out:;
}
@@ -340,16 +340,12 @@ int llc_conn_remove_acked_pdus(struct sock *sk, u8 nr, u16 *how_many_unacked)
/**
* llc_conn_send_pdus - Sends queued PDUs
* @sk: active connection
- * @hold_skb: the skb held by caller, or NULL if does not care
*
- * Sends queued pdus to MAC layer for transmission. When @hold_skb is
- * NULL, always return 0. Otherwise, return 0 if @hold_skb is sent
- * successfully, or 1 for failure.
+ * Sends queued pdus to MAC layer for transmission.
*/
-static int llc_conn_send_pdus(struct sock *sk, struct sk_buff *hold_skb)
+static void llc_conn_send_pdus(struct sock *sk)
{
struct sk_buff *skb;
- int ret = 0;
while ((skb = skb_dequeue(&sk->sk_write_queue)) != NULL) {
struct llc_pdu_sn *pdu = llc_pdu_sn_hdr(skb);
@@ -361,20 +357,10 @@ static int llc_conn_send_pdus(struct sock *sk, struct sk_buff *hold_skb)
skb_queue_tail(&llc_sk(sk)->pdu_unack_q, skb);
if (!skb2)
break;
- dev_queue_xmit(skb2);
- } else {
- bool is_target = skb == hold_skb;
- int rc;
-
- if (is_target)
- skb_get(skb);
- rc = dev_queue_xmit(skb);
- if (is_target)
- ret = rc;
+ skb = skb2;
}
+ dev_queue_xmit(skb);
}
-
- return ret;
}
/**
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH AUTOSEL 4.19 26/37] NFC: pn533: fix use-after-free and memleaks
[not found] <20191025135603.25093-1-sashal@kernel.org>
` (7 preceding siblings ...)
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 25/37] llc: fix sk_buff leak in llc_conn_service() Sasha Levin
@ 2019-10-25 13:55 ` Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 27/37] bonding: fix potential NULL deref in bond_update_slave_arr Sasha Levin
` (3 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Johan Hovold, syzbot+cb035c75c03dbe34b796, Jakub Kicinski,
Sasha Levin, netdev
From: Johan Hovold <johan@kernel.org>
[ Upstream commit 6af3aa57a0984e061f61308fe181a9a12359fecc ]
The driver would fail to deregister and its class device and free
related resources on late probe errors.
Reported-by: syzbot+cb035c75c03dbe34b796@syzkaller.appspotmail.com
Fixes: 32ecc75ded72 ("NFC: pn533: change order operations in dev registation")
Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/nfc/pn533/usb.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index 5d823e965883b..fcb57d64d97e6 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -559,18 +559,25 @@ static int pn533_usb_probe(struct usb_interface *interface,
rc = pn533_finalize_setup(priv);
if (rc)
- goto error;
+ goto err_deregister;
usb_set_intfdata(interface, phy);
return 0;
+err_deregister:
+ pn533_unregister_device(phy->priv);
error:
+ usb_kill_urb(phy->in_urb);
+ usb_kill_urb(phy->out_urb);
+ usb_kill_urb(phy->ack_urb);
+
usb_free_urb(phy->in_urb);
usb_free_urb(phy->out_urb);
usb_free_urb(phy->ack_urb);
usb_put_dev(phy->udev);
kfree(in_buf);
+ kfree(phy->ack_buffer);
return rc;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH AUTOSEL 4.19 27/37] bonding: fix potential NULL deref in bond_update_slave_arr
[not found] <20191025135603.25093-1-sashal@kernel.org>
` (8 preceding siblings ...)
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 26/37] NFC: pn533: fix use-after-free and memleaks Sasha Levin
@ 2019-10-25 13:55 ` Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 28/37] net: usb: sr9800: fix uninitialized local variable Sasha Levin
` (2 subsequent siblings)
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Eric Dumazet, syzbot, Mahesh Bandewar, Jakub Kicinski,
Sasha Levin, netdev
From: Eric Dumazet <edumazet@google.com>
[ Upstream commit a7137534b597b7c303203e6bc3ed87e87a273bb8 ]
syzbot got a NULL dereference in bond_update_slave_arr() [1],
happening after a failure to allocate bond->slave_arr
A workqueue (bond_slave_arr_handler) is supposed to retry
the allocation later, but if the slave is removed before
the workqueue had a chance to complete, bond->slave_arr
can still be NULL.
[1]
Failed to build slave-array.
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN PTI
Modules linked in:
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:bond_update_slave_arr.cold+0xc6/0x198 drivers/net/bonding/bond_main.c:4039
RSP: 0018:ffff88018fe33678 EFLAGS: 00010246
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffc9000290b000
RDX: 0000000000000000 RSI: ffffffff82b63037 RDI: ffff88019745ea20
RBP: ffff88018fe33760 R08: ffff880170754280 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: ffff88019745ea00 R14: 0000000000000000 R15: ffff88018fe338b0
FS: 00007febd837d700(0000) GS:ffff8801dad00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004540a0 CR3: 00000001c242e005 CR4: 00000000001626f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
[<ffffffff82b5b45e>] __bond_release_one+0x43e/0x500 drivers/net/bonding/bond_main.c:1923
[<ffffffff82b5b966>] bond_release drivers/net/bonding/bond_main.c:2039 [inline]
[<ffffffff82b5b966>] bond_do_ioctl+0x416/0x870 drivers/net/bonding/bond_main.c:3562
[<ffffffff83ae25f4>] dev_ifsioc+0x6f4/0x940 net/core/dev_ioctl.c:328
[<ffffffff83ae2e58>] dev_ioctl+0x1b8/0xc70 net/core/dev_ioctl.c:495
[<ffffffff83995ffd>] sock_do_ioctl+0x1bd/0x300 net/socket.c:1088
[<ffffffff83996a80>] sock_ioctl+0x300/0x5d0 net/socket.c:1196
[<ffffffff81b124db>] vfs_ioctl fs/ioctl.c:47 [inline]
[<ffffffff81b124db>] file_ioctl fs/ioctl.c:501 [inline]
[<ffffffff81b124db>] do_vfs_ioctl+0xacb/0x1300 fs/ioctl.c:688
[<ffffffff81b12dc6>] SYSC_ioctl fs/ioctl.c:705 [inline]
[<ffffffff81b12dc6>] SyS_ioctl+0xb6/0xe0 fs/ioctl.c:696
[<ffffffff8101ccc8>] do_syscall_64+0x528/0x770 arch/x86/entry/common.c:305
[<ffffffff84400091>] entry_SYSCALL_64_after_hwframe+0x42/0xb7
Fixes: ee6377147409 ("bonding: Simplify the xmit function for modes that use xmit_hash")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Cc: Mahesh Bandewar <maheshb@google.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/bonding/bond_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0d2392c4b625a..2804e2d1ae5e5 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4033,7 +4033,7 @@ int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
* this to-be-skipped slave to send a packet out.
*/
old_arr = rtnl_dereference(bond->slave_arr);
- for (idx = 0; idx < old_arr->count; idx++) {
+ for (idx = 0; old_arr != NULL && idx < old_arr->count; idx++) {
if (skipslave == old_arr->arr[idx]) {
old_arr->arr[idx] =
old_arr->arr[old_arr->count-1];
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH AUTOSEL 4.19 28/37] net: usb: sr9800: fix uninitialized local variable
[not found] <20191025135603.25093-1-sashal@kernel.org>
` (9 preceding siblings ...)
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 27/37] bonding: fix potential NULL deref in bond_update_slave_arr Sasha Levin
@ 2019-10-25 13:55 ` Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 30/37] ath9k: dynack: fix possible deadlock in ath_dynack_node_{de}init Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 35/37] ipv6: Handle race in addrconf_dad_work Sasha Levin
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Valentin Vidic, syzbot+f1842130bbcfb335bac1, David S . Miller,
Sasha Levin, linux-usb, netdev
From: Valentin Vidic <vvidic@valentin-vidic.from.hr>
[ Upstream commit 77b6d09f4ae66d42cd63b121af67780ae3d1a5e9 ]
Make sure res does not contain random value if the call to
sr_read_cmd fails for some reason.
Reported-by: syzbot+f1842130bbcfb335bac1@syzkaller.appspotmail.com
Signed-off-by: Valentin Vidic <vvidic@valentin-vidic.from.hr>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/usb/sr9800.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/sr9800.c b/drivers/net/usb/sr9800.c
index 35f39f23d8814..8f8c9ede88c26 100644
--- a/drivers/net/usb/sr9800.c
+++ b/drivers/net/usb/sr9800.c
@@ -336,7 +336,7 @@ static void sr_set_multicast(struct net_device *net)
static int sr_mdio_read(struct net_device *net, int phy_id, int loc)
{
struct usbnet *dev = netdev_priv(net);
- __le16 res;
+ __le16 res = 0;
mutex_lock(&dev->phy_mutex);
sr_set_sw_mii(dev);
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH AUTOSEL 4.19 30/37] ath9k: dynack: fix possible deadlock in ath_dynack_node_{de}init
[not found] <20191025135603.25093-1-sashal@kernel.org>
` (10 preceding siblings ...)
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 28/37] net: usb: sr9800: fix uninitialized local variable Sasha Levin
@ 2019-10-25 13:55 ` Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 35/37] ipv6: Handle race in addrconf_dad_work Sasha Levin
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Lorenzo Bianconi, Koen Vandeputte, Kalle Valo, Sasha Levin,
linux-wireless, netdev
From: Lorenzo Bianconi <lorenzo@kernel.org>
[ Upstream commit e1aa1a1db3b01c9890e82cf065cee99962ba1ed9 ]
Fix following lockdep warning disabling bh in
ath_dynack_node_init/ath_dynack_node_deinit
[ 75.955878] --------------------------------
[ 75.955880] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
[ 75.955884] swapper/0/0 [HC0[0]:SC1[3]:HE1:SE0] takes:
[ 75.955888] 00000000792a7ee0 (&(&da->qlock)->rlock){+.?.}, at: ath_dynack_sample_ack_ts+0x4d/0xa0 [ath9k_hw]
[ 75.955905] {SOFTIRQ-ON-W} state was registered at:
[ 75.955912] lock_acquire+0x9a/0x160
[ 75.955917] _raw_spin_lock+0x2c/0x70
[ 75.955927] ath_dynack_node_init+0x2a/0x60 [ath9k_hw]
[ 75.955934] ath9k_sta_state+0xec/0x160 [ath9k]
[ 75.955976] drv_sta_state+0xb2/0x740 [mac80211]
[ 75.956008] sta_info_insert_finish+0x21a/0x420 [mac80211]
[ 75.956039] sta_info_insert_rcu+0x12b/0x2c0 [mac80211]
[ 75.956069] sta_info_insert+0x7/0x70 [mac80211]
[ 75.956093] ieee80211_prep_connection+0x42e/0x730 [mac80211]
[ 75.956120] ieee80211_mgd_auth.cold+0xb9/0x15c [mac80211]
[ 75.956152] cfg80211_mlme_auth+0x143/0x350 [cfg80211]
[ 75.956169] nl80211_authenticate+0x25e/0x2b0 [cfg80211]
[ 75.956172] genl_family_rcv_msg+0x198/0x400
[ 75.956174] genl_rcv_msg+0x42/0x90
[ 75.956176] netlink_rcv_skb+0x35/0xf0
[ 75.956178] genl_rcv+0x1f/0x30
[ 75.956180] netlink_unicast+0x154/0x200
[ 75.956182] netlink_sendmsg+0x1bf/0x3d0
[ 75.956186] ___sys_sendmsg+0x2c2/0x2f0
[ 75.956187] __sys_sendmsg+0x44/0x80
[ 75.956190] do_syscall_64+0x55/0x1a0
[ 75.956192] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 75.956194] irq event stamp: 2357092
[ 75.956196] hardirqs last enabled at (2357092): [<ffffffff818c62de>] _raw_spin_unlock_irqrestore+0x3e/0x50
[ 75.956199] hardirqs last disabled at (2357091): [<ffffffff818c60b1>] _raw_spin_lock_irqsave+0x11/0x80
[ 75.956202] softirqs last enabled at (2357072): [<ffffffff8106dc09>] irq_enter+0x59/0x60
[ 75.956204] softirqs last disabled at (2357073): [<ffffffff8106dcbe>] irq_exit+0xae/0xc0
[ 75.956206]
other info that might help us debug this:
[ 75.956207] Possible unsafe locking scenario:
[ 75.956208] CPU0
[ 75.956209] ----
[ 75.956210] lock(&(&da->qlock)->rlock);
[ 75.956213] <Interrupt>
[ 75.956214] lock(&(&da->qlock)->rlock);
[ 75.956216]
*** DEADLOCK ***
[ 75.956217] 1 lock held by swapper/0/0:
[ 75.956219] #0: 000000003bb5675c (&(&sc->sc_pcu_lock)->rlock){+.-.}, at: ath9k_tasklet+0x55/0x240 [ath9k]
[ 75.956225]
stack backtrace:
[ 75.956228] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.3.0-rc1-wdn+ #13
[ 75.956229] Hardware name: Dell Inc. Studio XPS 1340/0K183D, BIOS A11 09/08/2009
[ 75.956231] Call Trace:
[ 75.956233] <IRQ>
[ 75.956236] dump_stack+0x67/0x90
[ 75.956239] mark_lock+0x4c1/0x640
[ 75.956242] ? check_usage_backwards+0x130/0x130
[ 75.956245] ? sched_clock_local+0x12/0x80
[ 75.956247] __lock_acquire+0x484/0x7a0
[ 75.956250] ? __lock_acquire+0x3b9/0x7a0
[ 75.956252] lock_acquire+0x9a/0x160
[ 75.956259] ? ath_dynack_sample_ack_ts+0x4d/0xa0 [ath9k_hw]
[ 75.956262] _raw_spin_lock_bh+0x34/0x80
[ 75.956268] ? ath_dynack_sample_ack_ts+0x4d/0xa0 [ath9k_hw]
[ 75.956275] ath_dynack_sample_ack_ts+0x4d/0xa0 [ath9k_hw]
[ 75.956280] ath_rx_tasklet+0xd09/0xe90 [ath9k]
[ 75.956286] ath9k_tasklet+0x102/0x240 [ath9k]
[ 75.956288] tasklet_action_common.isra.0+0x6d/0x170
[ 75.956291] __do_softirq+0xcc/0x425
[ 75.956294] irq_exit+0xae/0xc0
[ 75.956296] do_IRQ+0x8a/0x110
[ 75.956298] common_interrupt+0xf/0xf
[ 75.956300] </IRQ>
[ 75.956303] RIP: 0010:cpuidle_enter_state+0xb2/0x400
[ 75.956308] RSP: 0018:ffffffff82203e70 EFLAGS: 00000202 ORIG_RAX: ffffffffffffffd7
[ 75.956310] RAX: ffffffff82219800 RBX: ffffffff822bd0a0 RCX: 0000000000000000
[ 75.956312] RDX: 0000000000000046 RSI: 0000000000000006 RDI: ffffffff82219800
[ 75.956314] RBP: ffff888155a01c00 R08: 00000011af51aabe R09: 0000000000000000
[ 75.956315] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000002
[ 75.956317] R13: 00000011af51aabe R14: 0000000000000003 R15: ffffffff82219800
[ 75.956321] cpuidle_enter+0x24/0x40
[ 75.956323] do_idle+0x1ac/0x220
[ 75.956326] cpu_startup_entry+0x14/0x20
[ 75.956329] start_kernel+0x482/0x489
[ 75.956332] secondary_startup_64+0xa4/0xb0
Fixes: c774d57fd47c ("ath9k: add dynamic ACK timeout estimation")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
Tested-by: Koen Vandeputte <koen.vandeputte@ncentric.com>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/wireless/ath/ath9k/dynack.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/dynack.c b/drivers/net/wireless/ath/ath9k/dynack.c
index 6e236a4854311..71b4888b30e71 100644
--- a/drivers/net/wireless/ath/ath9k/dynack.c
+++ b/drivers/net/wireless/ath/ath9k/dynack.c
@@ -300,9 +300,9 @@ void ath_dynack_node_init(struct ath_hw *ah, struct ath_node *an)
an->ackto = ackto;
- spin_lock(&da->qlock);
+ spin_lock_bh(&da->qlock);
list_add_tail(&an->list, &da->nodes);
- spin_unlock(&da->qlock);
+ spin_unlock_bh(&da->qlock);
}
EXPORT_SYMBOL(ath_dynack_node_init);
@@ -316,9 +316,9 @@ void ath_dynack_node_deinit(struct ath_hw *ah, struct ath_node *an)
{
struct ath_dynack *da = &ah->dynack;
- spin_lock(&da->qlock);
+ spin_lock_bh(&da->qlock);
list_del(&an->list);
- spin_unlock(&da->qlock);
+ spin_unlock_bh(&da->qlock);
}
EXPORT_SYMBOL(ath_dynack_node_deinit);
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH AUTOSEL 4.19 35/37] ipv6: Handle race in addrconf_dad_work
[not found] <20191025135603.25093-1-sashal@kernel.org>
` (11 preceding siblings ...)
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 30/37] ath9k: dynack: fix possible deadlock in ath_dynack_node_{de}init Sasha Levin
@ 2019-10-25 13:55 ` Sasha Levin
12 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-10-25 13:55 UTC (permalink / raw)
To: linux-kernel, stable
Cc: David Ahern, Rajendra Dendukuri, Eric Dumazet, David S . Miller,
Sasha Levin, netdev
From: David Ahern <dsahern@gmail.com>
[ Upstream commit a3ce2a21bb8969ae27917281244fa91bf5f286d7 ]
Rajendra reported a kernel panic when a link was taken down:
[ 6870.263084] BUG: unable to handle kernel NULL pointer dereference at 00000000000000a8
[ 6870.271856] IP: [<ffffffff8efc5764>] __ipv6_ifa_notify+0x154/0x290
<snip>
[ 6870.570501] Call Trace:
[ 6870.573238] [<ffffffff8efc58c6>] ? ipv6_ifa_notify+0x26/0x40
[ 6870.579665] [<ffffffff8efc98ec>] ? addrconf_dad_completed+0x4c/0x2c0
[ 6870.586869] [<ffffffff8efe70c6>] ? ipv6_dev_mc_inc+0x196/0x260
[ 6870.593491] [<ffffffff8efc9c6a>] ? addrconf_dad_work+0x10a/0x430
[ 6870.600305] [<ffffffff8f01ade4>] ? __switch_to_asm+0x34/0x70
[ 6870.606732] [<ffffffff8ea93a7a>] ? process_one_work+0x18a/0x430
[ 6870.613449] [<ffffffff8ea93d6d>] ? worker_thread+0x4d/0x490
[ 6870.619778] [<ffffffff8ea93d20>] ? process_one_work+0x430/0x430
[ 6870.626495] [<ffffffff8ea99dd9>] ? kthread+0xd9/0xf0
[ 6870.632145] [<ffffffff8f01ade4>] ? __switch_to_asm+0x34/0x70
[ 6870.638573] [<ffffffff8ea99d00>] ? kthread_park+0x60/0x60
[ 6870.644707] [<ffffffff8f01ae77>] ? ret_from_fork+0x57/0x70
[ 6870.650936] Code: 31 c0 31 d2 41 b9 20 00 08 02 b9 09 00 00 0
addrconf_dad_work is kicked to be scheduled when a device is brought
up. There is a race between addrcond_dad_work getting scheduled and
taking the rtnl lock and a process taking the link down (under rtnl).
The latter removes the host route from the inet6_addr as part of
addrconf_ifdown which is run for NETDEV_DOWN. The former attempts
to use the host route in ipv6_ifa_notify. If the down event removes
the host route due to the race to the rtnl, then the BUG listed above
occurs.
This scenario does not occur when the ipv6 address is not kept
(net.ipv6.conf.all.keep_addr_on_down = 0) as addrconf_ifdown sets the
state of the ifp to DEAD. Handle when the addresses are kept by checking
IF_READY which is reset by addrconf_ifdown.
The 'dead' flag for an inet6_addr is set only under rtnl, in
addrconf_ifdown and it means the device is getting removed (or IPv6 is
disabled). The interesting cases for changing the idev flag are
addrconf_notify (NETDEV_UP and NETDEV_CHANGE) and addrconf_ifdown
(reset the flag). The former does not have the idev lock - only rtnl;
the latter has both. Based on that the existing dead + IF_READY check
can be moved to right after the rtnl_lock in addrconf_dad_work.
Fixes: f1705ec197e7 ("net: ipv6: Make address flushing on ifdown optional")
Reported-by: Rajendra Dendukuri <rajendra.dendukuri@broadcom.com>
Signed-off-by: David Ahern <dsahern@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/ipv6/addrconf.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index d2968a79abea8..75f520db6a6db 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3974,6 +3974,12 @@ static void addrconf_dad_work(struct work_struct *w)
rtnl_lock();
+ /* check if device was taken down before this delayed work
+ * function could be canceled
+ */
+ if (idev->dead || !(idev->if_flags & IF_READY))
+ goto out;
+
spin_lock_bh(&ifp->lock);
if (ifp->state == INET6_IFADDR_STATE_PREDAD) {
action = DAD_BEGIN;
@@ -4019,11 +4025,6 @@ static void addrconf_dad_work(struct work_struct *w)
goto out;
write_lock_bh(&idev->lock);
- if (idev->dead || !(idev->if_flags & IF_READY)) {
- write_unlock_bh(&idev->lock);
- goto out;
- }
-
spin_lock(&ifp->lock);
if (ifp->state == INET6_IFADDR_STATE_DEAD) {
spin_unlock(&ifp->lock);
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-10-25 14:04 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20191025135603.25093-1-sashal@kernel.org>
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 16/37] xfrm interface: fix memory leak on creation Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 19/37] net: ipv6: fix listify ip6_rcv_finish in case of forwarding Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 20/37] sch_netem: fix rcu splat in netem_enqueue() Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 21/37] rxrpc: Fix call ref leak Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 22/37] rxrpc: Fix trace-after-put looking at the put peer record Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 23/37] rxrpc: rxrpc_peer needs to hold a ref on the rxrpc_local record Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 24/37] llc: fix sk_buff leak in llc_sap_state_process() Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 25/37] llc: fix sk_buff leak in llc_conn_service() Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 26/37] NFC: pn533: fix use-after-free and memleaks Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 27/37] bonding: fix potential NULL deref in bond_update_slave_arr Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 28/37] net: usb: sr9800: fix uninitialized local variable Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 30/37] ath9k: dynack: fix possible deadlock in ath_dynack_node_{de}init Sasha Levin
2019-10-25 13:55 ` [PATCH AUTOSEL 4.19 35/37] ipv6: Handle race in addrconf_dad_work Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).