* [Patch net 0/3] hsr: a few bug fixes
@ 2019-07-04 0:21 Cong Wang
2019-07-04 0:21 ` [Patch net 1/3] hsr: fix a memory leak in hsr_del_port() Cong Wang
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Cong Wang @ 2019-07-04 0:21 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Arvid Brodin
This patchset contains 3 bug fixes for hsr triggered by a syzbot
reproducer, please check each patch for details.
Cc: Arvid Brodin <arvid.brodin@alten.se>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Cong Wang (3):
hsr: fix a memory leak in hsr_del_port()
hsr: implement dellink to clean up resources
hsr: fix a NULL pointer deref in hsr_dev_xmit()
---
net/hsr/hsr_device.c | 29 ++++++++++++++++-------------
net/hsr/hsr_device.h | 1 +
net/hsr/hsr_framereg.c | 11 ++++++++++-
net/hsr/hsr_framereg.h | 3 ++-
net/hsr/hsr_netlink.c | 7 +++++++
net/hsr/hsr_slave.c | 1 +
6 files changed, 37 insertions(+), 15 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Patch net 1/3] hsr: fix a memory leak in hsr_del_port()
2019-07-04 0:21 [Patch net 0/3] hsr: a few bug fixes Cong Wang
@ 2019-07-04 0:21 ` Cong Wang
2019-07-04 0:21 ` [Patch net 2/3] hsr: implement dellink to clean up resources Cong Wang
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2019-07-04 0:21 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Arvid Brodin
hsr_del_port() should release all the resources allocated
in hsr_add_port().
As a consequence of this change, hsr_for_each_port() is no
longer safe to work with hsr_del_port(), switch to
list_for_each_entry_safe() as we always hold RTNL lock.
Cc: Arvid Brodin <arvid.brodin@alten.se>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/hsr/hsr_device.c | 6 ++++--
net/hsr/hsr_slave.c | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 15c72065df79..f48b6a275cf0 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -351,13 +351,14 @@ static void hsr_dev_destroy(struct net_device *hsr_dev)
{
struct hsr_priv *hsr;
struct hsr_port *port;
+ struct hsr_port *tmp;
hsr = netdev_priv(hsr_dev);
hsr_debugfs_term(hsr);
rtnl_lock();
- hsr_for_each_port(hsr, port)
+ list_for_each_entry_safe(port, tmp, &hsr->ports, port_list)
hsr_del_port(port);
rtnl_unlock();
@@ -428,6 +429,7 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
{
struct hsr_priv *hsr;
struct hsr_port *port;
+ struct hsr_port *tmp;
int res;
hsr = netdev_priv(hsr_dev);
@@ -492,7 +494,7 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
return 0;
fail:
- hsr_for_each_port(hsr, port)
+ list_for_each_entry_safe(port, tmp, &hsr->ports, port_list)
hsr_del_port(port);
err_add_port:
hsr_del_node(&hsr->self_node_db);
diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
index 88b6705ded83..ee561297d8a7 100644
--- a/net/hsr/hsr_slave.c
+++ b/net/hsr/hsr_slave.c
@@ -193,4 +193,5 @@ void hsr_del_port(struct hsr_port *port)
if (port != master)
dev_put(port->dev);
+ kfree(port);
}
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Patch net 2/3] hsr: implement dellink to clean up resources
2019-07-04 0:21 [Patch net 0/3] hsr: a few bug fixes Cong Wang
2019-07-04 0:21 ` [Patch net 1/3] hsr: fix a memory leak in hsr_del_port() Cong Wang
@ 2019-07-04 0:21 ` Cong Wang
2019-07-04 0:21 ` [Patch net 3/3] hsr: fix a NULL pointer deref in hsr_dev_xmit() Cong Wang
2019-07-05 22:22 ` [Patch net 0/3] hsr: a few bug fixes David Miller
3 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2019-07-04 0:21 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, syzbot+c6167ec3de7def23d1e8, Arvid Brodin
hsr_link_ops implements ->newlink() but not ->dellink(),
which leads that resources not released after removing the device,
particularly the entries in self_node_db and node_db.
So add ->dellink() implementation to replace the priv_destructor.
This also makes the code slightly easier to understand.
Reported-by: syzbot+c6167ec3de7def23d1e8@syzkaller.appspotmail.com
Cc: Arvid Brodin <arvid.brodin@alten.se>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/hsr/hsr_device.c | 13 +++++--------
net/hsr/hsr_device.h | 1 +
net/hsr/hsr_framereg.c | 11 ++++++++++-
net/hsr/hsr_framereg.h | 3 ++-
net/hsr/hsr_netlink.c | 7 +++++++
5 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index f48b6a275cf0..4ea7d54a8262 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -344,10 +344,7 @@ static void hsr_announce(struct timer_list *t)
rcu_read_unlock();
}
-/* According to comments in the declaration of struct net_device, this function
- * is "Called from unregister, can be used to call free_netdev". Ok then...
- */
-static void hsr_dev_destroy(struct net_device *hsr_dev)
+void hsr_dev_destroy(struct net_device *hsr_dev)
{
struct hsr_priv *hsr;
struct hsr_port *port;
@@ -357,15 +354,16 @@ static void hsr_dev_destroy(struct net_device *hsr_dev)
hsr_debugfs_term(hsr);
- rtnl_lock();
list_for_each_entry_safe(port, tmp, &hsr->ports, port_list)
hsr_del_port(port);
- rtnl_unlock();
del_timer_sync(&hsr->prune_timer);
del_timer_sync(&hsr->announce_timer);
synchronize_rcu();
+
+ hsr_del_self_node(&hsr->self_node_db);
+ hsr_del_nodes(&hsr->node_db);
}
static const struct net_device_ops hsr_device_ops = {
@@ -392,7 +390,6 @@ void hsr_dev_setup(struct net_device *dev)
dev->priv_flags |= IFF_NO_QUEUE;
dev->needs_free_netdev = true;
- dev->priv_destructor = hsr_dev_destroy;
dev->hw_features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HIGHDMA |
NETIF_F_GSO_MASK | NETIF_F_HW_CSUM |
@@ -497,7 +494,7 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
list_for_each_entry_safe(port, tmp, &hsr->ports, port_list)
hsr_del_port(port);
err_add_port:
- hsr_del_node(&hsr->self_node_db);
+ hsr_del_self_node(&hsr->self_node_db);
return res;
}
diff --git a/net/hsr/hsr_device.h b/net/hsr/hsr_device.h
index 6d7759c4f5f9..d0fa6b0696d2 100644
--- a/net/hsr/hsr_device.h
+++ b/net/hsr/hsr_device.h
@@ -14,6 +14,7 @@
void hsr_dev_setup(struct net_device *dev);
int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
unsigned char multicast_spec, u8 protocol_version);
+void hsr_dev_destroy(struct net_device *hsr_dev);
void hsr_check_carrier_and_operstate(struct hsr_priv *hsr);
bool is_hsr_master(struct net_device *dev);
int hsr_get_max_mtu(struct hsr_priv *hsr);
diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
index 2d7a19750436..292be446007b 100644
--- a/net/hsr/hsr_framereg.c
+++ b/net/hsr/hsr_framereg.c
@@ -104,7 +104,7 @@ int hsr_create_self_node(struct list_head *self_node_db,
return 0;
}
-void hsr_del_node(struct list_head *self_node_db)
+void hsr_del_self_node(struct list_head *self_node_db)
{
struct hsr_node *node;
@@ -117,6 +117,15 @@ void hsr_del_node(struct list_head *self_node_db)
}
}
+void hsr_del_nodes(struct list_head *node_db)
+{
+ struct hsr_node *node;
+ struct hsr_node *tmp;
+
+ list_for_each_entry_safe(node, tmp, node_db, mac_list)
+ kfree(node);
+}
+
/* Allocate an hsr_node and add it to node_db. 'addr' is the node's address_A;
* seq_out is used to initialize filtering of outgoing duplicate frames
* originating from the newly added node.
diff --git a/net/hsr/hsr_framereg.h b/net/hsr/hsr_framereg.h
index a3bdcdab469d..89a3ce38151d 100644
--- a/net/hsr/hsr_framereg.h
+++ b/net/hsr/hsr_framereg.h
@@ -12,7 +12,8 @@
struct hsr_node;
-void hsr_del_node(struct list_head *self_node_db);
+void hsr_del_self_node(struct list_head *self_node_db);
+void hsr_del_nodes(struct list_head *node_db);
struct hsr_node *hsr_add_node(struct list_head *node_db, unsigned char addr[],
u16 seq_out);
struct hsr_node *hsr_get_node(struct hsr_port *port, struct sk_buff *skb,
diff --git a/net/hsr/hsr_netlink.c b/net/hsr/hsr_netlink.c
index 8f8337f893ba..160edd24de4e 100644
--- a/net/hsr/hsr_netlink.c
+++ b/net/hsr/hsr_netlink.c
@@ -69,6 +69,12 @@ static int hsr_newlink(struct net *src_net, struct net_device *dev,
return hsr_dev_finalize(dev, link, multicast_spec, hsr_version);
}
+static void hsr_dellink(struct net_device *hsr_dev, struct list_head *head)
+{
+ hsr_dev_destroy(hsr_dev);
+ unregister_netdevice_queue(hsr_dev, head);
+}
+
static int hsr_fill_info(struct sk_buff *skb, const struct net_device *dev)
{
struct hsr_priv *hsr;
@@ -113,6 +119,7 @@ static struct rtnl_link_ops hsr_link_ops __read_mostly = {
.priv_size = sizeof(struct hsr_priv),
.setup = hsr_dev_setup,
.newlink = hsr_newlink,
+ .dellink = hsr_dellink,
.fill_info = hsr_fill_info,
};
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Patch net 3/3] hsr: fix a NULL pointer deref in hsr_dev_xmit()
2019-07-04 0:21 [Patch net 0/3] hsr: a few bug fixes Cong Wang
2019-07-04 0:21 ` [Patch net 1/3] hsr: fix a memory leak in hsr_del_port() Cong Wang
2019-07-04 0:21 ` [Patch net 2/3] hsr: implement dellink to clean up resources Cong Wang
@ 2019-07-04 0:21 ` Cong Wang
2019-07-05 22:22 ` [Patch net 0/3] hsr: a few bug fixes David Miller
3 siblings, 0 replies; 5+ messages in thread
From: Cong Wang @ 2019-07-04 0:21 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Arvid Brodin
hsr_port_get_hsr() could return NULL and kernel
could crash:
BUG: kernel NULL pointer dereference, address: 0000000000000010
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 8000000074b84067 P4D 8000000074b84067 PUD 7057d067 PMD 0
Oops: 0000 [#1] SMP PTI
CPU: 0 PID: 754 Comm: a.out Not tainted 5.2.0-rc6+ #718
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-2.fc30 04/01/2014
RIP: 0010:hsr_dev_xmit+0x20/0x31
Code: 48 8b 1b eb e0 5b 5d 41 5c c3 66 66 66 66 90 55 48 89 fd 48 8d be 40 0b 00 00 be 04 00 00 00 e8 ee f2 ff ff 48 89 ef 48 89 c6 <48> 8b 40 10 48 89 45 10 e8 6c 1b 00 00 31 c0 5d c3 66 66 66 66 90
RSP: 0018:ffffb5b400003c48 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff9821b4509a88 RCX: 0000000000000000
RDX: ffff9821b4509a88 RSI: 0000000000000000 RDI: ffff9821bc3fc7c0
RBP: ffff9821bc3fc7c0 R08: 0000000000000000 R09: 00000000000c2019
R10: 0000000000000000 R11: 0000000000000002 R12: ffff9821bc3fc7c0
R13: ffff9821b4509a88 R14: 0000000000000000 R15: 000000000000006e
FS: 00007fee112a1800(0000) GS:ffff9821bd800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000010 CR3: 000000006e9ce000 CR4: 00000000000406f0
Call Trace:
<IRQ>
netdev_start_xmit+0x1b/0x38
dev_hard_start_xmit+0x121/0x21e
? validate_xmit_skb.isra.0+0x19/0x1e3
__dev_queue_xmit+0x74c/0x823
? lockdep_hardirqs_on+0x12b/0x17d
ip6_finish_output2+0x3d3/0x42c
? ip6_mtu+0x55/0x5c
? mld_sendpack+0x191/0x229
mld_sendpack+0x191/0x229
mld_ifc_timer_expire+0x1f7/0x230
? mld_dad_timer_expire+0x58/0x58
call_timer_fn+0x12e/0x273
__run_timers.part.0+0x174/0x1b5
? mld_dad_timer_expire+0x58/0x58
? sched_clock_cpu+0x10/0xad
? mark_lock+0x26/0x1f2
? __lock_is_held+0x40/0x71
run_timer_softirq+0x26/0x48
__do_softirq+0x1af/0x392
irq_exit+0x53/0xa2
smp_apic_timer_interrupt+0x1c4/0x1d9
apic_timer_interrupt+0xf/0x20
</IRQ>
Cc: Arvid Brodin <arvid.brodin@alten.se>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/hsr/hsr_device.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 4ea7d54a8262..f0f9b493c47b 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -227,9 +227,13 @@ static int hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
struct hsr_port *master;
master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
- skb->dev = master->dev;
- hsr_forward_skb(skb, master);
-
+ if (master) {
+ skb->dev = master->dev;
+ hsr_forward_skb(skb, master);
+ } else {
+ atomic_long_inc(&dev->tx_dropped);
+ dev_kfree_skb_any(skb);
+ }
return NETDEV_TX_OK;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Patch net 0/3] hsr: a few bug fixes
2019-07-04 0:21 [Patch net 0/3] hsr: a few bug fixes Cong Wang
` (2 preceding siblings ...)
2019-07-04 0:21 ` [Patch net 3/3] hsr: fix a NULL pointer deref in hsr_dev_xmit() Cong Wang
@ 2019-07-05 22:22 ` David Miller
3 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2019-07-05 22:22 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, arvid.brodin
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Wed, 3 Jul 2019 17:21:11 -0700
> This patchset contains 3 bug fixes for hsr triggered by a syzbot
> reproducer, please check each patch for details.
>
> Cc: Arvid Brodin <arvid.brodin@alten.se>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Series applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-05 22:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-04 0:21 [Patch net 0/3] hsr: a few bug fixes Cong Wang
2019-07-04 0:21 ` [Patch net 1/3] hsr: fix a memory leak in hsr_del_port() Cong Wang
2019-07-04 0:21 ` [Patch net 2/3] hsr: implement dellink to clean up resources Cong Wang
2019-07-04 0:21 ` [Patch net 3/3] hsr: fix a NULL pointer deref in hsr_dev_xmit() Cong Wang
2019-07-05 22:22 ` [Patch net 0/3] hsr: a few bug fixes David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.