[PATCH v1] drbd: fix use-after-frees of disk_conf and rs_plan_s in chg-disk-opts

Binbin Deng posted 1 patch 1 day, 23 hours ago
drivers/block/drbd/drbd_nl.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
[PATCH v1] drbd: fix use-after-frees of disk_conf and rs_plan_s in chg-disk-opts
Posted by Binbin Deng 1 day, 23 hours ago
drbd_nl_chg_disk_opts_doit() installs a new disk_conf, drops conf_update
and then keeps reading from the old and the new object. A concurrent
receive_SyncParam(), which runs from the receiver thread and serialises
on the same mutex, may install another disk_conf and kfree() the published
one as soon as conf_update is released.  Its synchronize_rcu() does not
wait for these reads, because they are not inside an RCU read-side
critical section, so the object can be freed while it is still being read.

BUG: KASAN: slab-use-after-free in drbd_nl_chg_disk_opts_doit+0x16a8/0x18b0
Read of size 1 at addr ffff888100d84950 by task drbdsetup-84/100447
Call Trace:
 <TASK>
 dump_stack_lvl+0x53/0x70
 print_report+0xd0/0x630
 ? __pfx__raw_spin_lock_irqsave+0x10/0x10
 ? drbd_nl_chg_disk_opts_doit+0x16a8/0x18b0
 ? genlmsg_put+0x12f/0x330
 ? __pfx_drbd_nl_chg_disk_opts_doit+0x10/0x10
 ? __nla_parse+0x24/0x30
 ? genl_family_rcv_msg_attrs_parse.isra.0+0x17a/0x290
 genl_family_rcv_msg_doit+0x1e0/0x2c0
 ? __pfx_genl_family_rcv_msg_doit+0x10/0x10
 ? __pfx_cred_has_capability.isra.0+0x10/0x10
 genl_rcv_msg+0x419/0x6d0
 ? __pfx_genl_rcv_msg+0x10/0x10
 ? __pfx_drbd_pre_doit+0x10/0x10
 ? __pfx_drbd_nl_chg_disk_opts_doit+0x10/0x10
 ? __pfx_drbd_post_doit+0x10/0x10
 netlink_rcv_skb+0x11f/0x350
 ? __pfx_genl_rcv_msg+0x10/0x10
 ? __pfx_netlink_rcv_skb+0x10/0x10
 ? __pfx___netlink_lookup+0x10/0x10
 genl_rcv+0x23/0x30
 netlink_unicast+0x5f5/0x860
 ? __pfx_netlink_unicast+0x10/0x10
 ? __pfx_sock_has_perm+0x10/0x10
 netlink_sendmsg+0x70a/0xba0
 ? __pfx_netlink_sendmsg+0x10/0x10
 ? selinux_socket_sendmsg+0x8c/0x270
 sock_write_iter+0x405/0x4a0
 ? __pfx_sock_write_iter+0x10/0x10
 ? __file_has_perm+0x30f/0x410
 ? folio_add_new_anon_rmap+0x254/0x5d0
 vfs_write+0xa4b/0xcf0
 ? __handle_mm_fault+0xd1a/0x1e30
 ? __pfx_vfs_write+0x10/0x10
 ? fdget_pos+0x53/0x4b0
 ksys_write+0x17c/0x1c0
 ? __pfx_ksys_write+0x10/0x10
 do_syscall_64+0xf9/0x540
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
 </TASK>

Fix this by caching the values needed from the disk_conf objects while
conf_update is still held, and by waiting for a grace period before
freeing the previous rs_plan_s.

Fixes: 813472ced7fac ("drbd: RCU for rs_plan_s")
Signed-off-by: Binbin Deng <18983559317@163.com>
---
 drivers/block/drbd/drbd_nl.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index b77f901fc3ef..f34ed481217e 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -1657,7 +1657,6 @@ int drbd_nl_chg_disk_opts_doit(struct sk_buff *skb, struct genl_info *info)
 		old_plan = device->rs_plan_s;
 		rcu_assign_pointer(device->rs_plan_s, new_plan);
 	}
-
 	mutex_unlock(&device->resource->conf_update);
 
 	if (new_disk_conf->al_updates)
@@ -1687,7 +1686,16 @@ int drbd_nl_chg_disk_opts_doit(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	kvfree_rcu_mightsleep(old_disk_conf);
-	kfree(old_plan);
+	if (old_plan) {
+		/*
+		 * rs_plan_s is dereferenced by the resync controller
+		 * (drbd_rs_controller()) under rcu_read_lock(), so wait for
+		 * a grace period before releasing the previous plan, like
+		 * receive_SyncParam() does.
+		 */
+		synchronize_rcu();
+		kfree(old_plan);
+	}
 	mod_timer(&device->request_timer, jiffies + HZ);
 	goto success;
 
-- 
2.43.0