Menu
▾
▴
[tipc-discussion] [net-next v2 2/2] tipc: fix use-after-free in tipc_bcast_get_mode
|
From: Hoang H. Le <hoa...@de...> - 2020-09-03 12:03:46
|
Syzbot has reported those issues as:
==================================================================
BUG: KASAN: use-after-free in tipc_bcast_get_mode+0x3ab/0x400 net/tipc/bcast.c:759
Read of size 1 at addr ffff88805e6b3571 by task kworker/0:6/3850
CPU: 0 PID: 3850 Comm: kworker/0:6 Not tainted 5.8.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: events tipc_net_finalize_work
Thread 1's call trace:
[...]
kfree+0x103/0x2c0 mm/slab.c:3757 <- bcbase releasing
tipc_bcast_stop+0x1b0/0x2f0 net/tipc/bcast.c:721
tipc_exit_net+0x24/0x270 net/tipc/core.c:112
[...]
Thread 2's call trace:
[...]
tipc_bcast_get_mode+0x3ab/0x400 net/tipc/bcast.c:759 <- bcbase
has already been freed by Thread 1
tipc_node_broadcast+0x9e/0xcc0 net/tipc/node.c:1744
tipc_nametbl_publish+0x60b/0x970 net/tipc/name_table.c:752
tipc_net_finalize net/tipc/net.c:141 [inline]
tipc_net_finalize+0x1fa/0x310 net/tipc/net.c:131
tipc_net_finalize_work+0x55/0x80 net/tipc/net.c:150
[...]
==================================================================
BUG: KASAN: use-after-free in tipc_named_reinit+0xef/0x290 net/tipc/name_distr.c:344
Read of size 8 at addr ffff888052ab2000 by task kworker/0:13/30628
CPU: 0 PID: 30628 Comm: kworker/0:13 Not tainted 5.8.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: events tipc_net_finalize_work
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1f0/0x31e lib/dump_stack.c:118
print_address_description+0x66/0x5a0 mm/kasan/report.c:383
__kasan_report mm/kasan/report.c:513 [inline]
kasan_report+0x132/0x1d0 mm/kasan/report.c:530
tipc_named_reinit+0xef/0x290 net/tipc/name_distr.c:344
tipc_net_finalize+0x85/0xe0 net/tipc/net.c:138
tipc_net_finalize_work+0x50/0x70 net/tipc/net.c:150
process_one_work+0x789/0xfc0 kernel/workqueue.c:2269
worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
kthread+0x37e/0x3a0 drivers/block/aoe/aoecmd.c:1234
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293
[...]
Freed by task 14058:
save_stack mm/kasan/common.c:48 [inline]
set_track mm/kasan/common.c:56 [inline]
kasan_set_free_info mm/kasan/common.c:316 [inline]
__kasan_slab_free+0x114/0x170 mm/kasan/common.c:455
__cache_free mm/slab.c:3426 [inline]
kfree+0x10a/0x220 mm/slab.c:3757
tipc_exit_net+0x29/0x50 net/tipc/core.c:113
ops_exit_list net/core/net_namespace.c:186 [inline]
cleanup_net+0x708/0xba0 net/core/net_namespace.c:603
process_one_work+0x789/0xfc0 kernel/workqueue.c:2269
worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
kthread+0x37e/0x3a0 drivers/block/aoe/aoecmd.c:1234
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293
Fix it by calling cancel_work_sync() to make sure the
tipc_net_finalize_work() finished before releasing bcbase object.
Reported-by: syz...@sy...
Reported-by: syz...@sy...
Signed-off-by: Hoang Huu Le <hoa...@de...>
---
net/tipc/core.c | 6 ++++++
net/tipc/core.h | 10 ++++++++++
net/tipc/net.c | 21 +++++++++------------
3 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/net/tipc/core.c b/net/tipc/core.c
index 4f6dc74adf45..f81b999980e7 100644
--- a/net/tipc/core.c
+++ b/net/tipc/core.c
@@ -60,6 +60,7 @@ static int __net_init tipc_init_net(struct net *net)
tn->trial_addr = 0;
tn->addr_trial_end = 0;
tn->capabilities = TIPC_NODE_CAPABILITIES;
+ clear_bit_unlock(0, &tn->final_work_flag);
memset(tn->node_id, 0, sizeof(tn->node_id));
memset(tn->node_id_string, 0, sizeof(tn->node_id_string));
tn->mon_threshold = TIPC_DEF_MON_THRESHOLD;
@@ -119,7 +120,12 @@ static void __net_exit tipc_exit_net(struct net *net)
static void __net_exit tipc_pernet_pre_exit(struct net *net)
{
+ struct tipc_net *tn = tipc_net(net);
+
tipc_node_pre_cleanup_net(net);
+ /* Make sure the tipc_net_finalize_work() finished */
+ if (likely(test_bit(0, &tn->final_work_flag)))
+ cancel_work_sync(&tn->final_work.work);
}
static struct pernet_operations tipc_pernet_pre_exit_ops = {
diff --git a/net/tipc/core.h b/net/tipc/core.h
index 631d83c9705f..396105f17bc9 100644
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -90,6 +90,12 @@ extern unsigned int tipc_net_id __read_mostly;
extern int sysctl_tipc_rmem[3] __read_mostly;
extern int sysctl_tipc_named_timeout __read_mostly;
+struct tipc_net_work {
+ struct work_struct work;
+ struct net *net;
+ u32 addr;
+};
+
struct tipc_net {
u8 node_id[NODE_ID_LEN];
u32 node_addr;
@@ -143,6 +149,10 @@ struct tipc_net {
/* TX crypto handler */
struct tipc_crypto *crypto_tx;
#endif
+ /* Work item for finalize */
+ struct tipc_net_work final_work;
+ /* Flag to indicate final_work queued */
+ unsigned long final_work_flag;
};
static inline struct tipc_net *tipc_net(struct net *net)
diff --git a/net/tipc/net.c b/net/tipc/net.c
index 85400e4242de..580c542531b4 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c
@@ -105,11 +105,6 @@
* - A local spin_lock protecting the queue of subscriber events.
*/
-struct tipc_net_work {
- struct work_struct work;
- struct net *net;
- u32 addr;
-};
static void tipc_net_finalize(struct net *net, u32 addr);
@@ -140,6 +135,7 @@ static void tipc_net_finalize(struct net *net, u32 addr)
tipc_mon_reinit_self(net);
tipc_nametbl_publish(net, TIPC_CFG_SRV, addr, addr,
TIPC_CLUSTER_SCOPE, 0, addr);
+ clear_bit_unlock(0, &tn->final_work_flag);
}
static void tipc_net_finalize_work(struct work_struct *work)
@@ -148,19 +144,20 @@ static void tipc_net_finalize_work(struct work_struct *work)
fwork = container_of(work, struct tipc_net_work, work);
tipc_net_finalize(fwork->net, fwork->addr);
- kfree(fwork);
}
void tipc_sched_net_finalize(struct net *net, u32 addr)
{
- struct tipc_net_work *fwork = kzalloc(sizeof(*fwork), GFP_ATOMIC);
+ struct tipc_net *tn = tipc_net(net);
- if (!fwork)
+ if (likely(test_bit(0, &tn->final_work_flag)))
return;
- INIT_WORK(&fwork->work, tipc_net_finalize_work);
- fwork->net = net;
- fwork->addr = addr;
- schedule_work(&fwork->work);
+
+ INIT_WORK(&tn->final_work.work, tipc_net_finalize_work);
+ tn->final_work.net = net;
+ tn->final_work.addr = addr;
+ test_and_set_bit_lock(0, &tn->final_work_flag);
+ schedule_work(&tn->final_work.work);
}
void tipc_net_stop(struct net *net)
--
2.25.1
|
