From 24cdb8f9436b89debfe665809c76ad65664baf1f Mon Sep 17 00:00:00 2001 From: Sasha Levin Date: Sun, 2 Feb 2020 22:01:13 -0500 Subject: [PATCH] fixes for 4.19 Signed-off-by: Sasha Levin --- ...fix-user-after-free-on-module-unload.patch | 43 ++++ ...-detach-for-hibernation-and-poweroff.patch | 87 +++++++ ...fter-free-on-failed-probe-and-unbind.patch | 76 ++++++ queue-4.19/series | 6 + ...a-deadlock-due-to-inaccurate-referen.patch | 230 +++++++++++++++++ ...use-after-free-due-to-inaccurate-ref.patch | 137 ++++++++++ ...use-after-free-when-deleting-resourc.patch | 234 ++++++++++++++++++ 7 files changed, 813 insertions(+) create mode 100644 queue-4.19/crypto-pcrypt-fix-user-after-free-on-module-unload.patch create mode 100644 queue-4.19/rsi-add-hci-detach-for-hibernation-and-poweroff.patch create mode 100644 queue-4.19/rsi-fix-use-after-free-on-failed-probe-and-unbind.patch create mode 100644 queue-4.19/x86-resctrl-fix-a-deadlock-due-to-inaccurate-referen.patch create mode 100644 queue-4.19/x86-resctrl-fix-use-after-free-due-to-inaccurate-ref.patch create mode 100644 queue-4.19/x86-resctrl-fix-use-after-free-when-deleting-resourc.patch diff --git a/queue-4.19/crypto-pcrypt-fix-user-after-free-on-module-unload.patch b/queue-4.19/crypto-pcrypt-fix-user-after-free-on-module-unload.patch new file mode 100644 index 00000000000..2bbcf7544e9 --- /dev/null +++ b/queue-4.19/crypto-pcrypt-fix-user-after-free-on-module-unload.patch @@ -0,0 +1,43 @@ +From e33f9806dcd82f870155f1b81efd3f7abf31da3b Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Tue, 19 Nov 2019 17:41:31 +0800 +Subject: crypto: pcrypt - Fix user-after-free on module unload + +From: Herbert Xu + +[ Upstream commit 07bfd9bdf568a38d9440c607b72342036011f727 ] + +On module unload of pcrypt we must unregister the crypto algorithms +first and then tear down the padata structure. As otherwise the +crypto algorithms are still alive and can be used while the padata +structure is being freed. + +Fixes: 5068c7a883d1 ("crypto: pcrypt - Add pcrypt crypto...") +Cc: +Signed-off-by: Herbert Xu +Signed-off-by: Sasha Levin +--- + crypto/pcrypt.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c +index a5718c0a3dc4e..1348541da463a 100644 +--- a/crypto/pcrypt.c ++++ b/crypto/pcrypt.c +@@ -505,11 +505,12 @@ static int __init pcrypt_init(void) + + static void __exit pcrypt_exit(void) + { ++ crypto_unregister_template(&pcrypt_tmpl); ++ + pcrypt_fini_padata(&pencrypt); + pcrypt_fini_padata(&pdecrypt); + + kset_unregister(pcrypt_kset); +- crypto_unregister_template(&pcrypt_tmpl); + } + + module_init(pcrypt_init); +-- +2.20.1 + diff --git a/queue-4.19/rsi-add-hci-detach-for-hibernation-and-poweroff.patch b/queue-4.19/rsi-add-hci-detach-for-hibernation-and-poweroff.patch new file mode 100644 index 00000000000..f5e4f6825b9 --- /dev/null +++ b/queue-4.19/rsi-add-hci-detach-for-hibernation-and-poweroff.patch @@ -0,0 +1,87 @@ +From e24264ea1cfb317a8718df66ba9e43eab5e39809 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Mon, 4 Feb 2019 12:03:26 +0530 +Subject: rsi: add hci detach for hibernation and poweroff + +From: Siva Rebbagondla + +[ Upstream commit cbde979b33fa16e06dadc2c81093699a2bc787db ] + +As we missed to detach HCI, while entering power off or hibernation, +an extra hci interface gets created whenever system is woken up, to +avoid this we added hci_detach() in rsi_disconnect(), rsi_freeze(), +and rsi_shutdown() functions which are invoked for these tests. +This patch fixes the issue + +Signed-off-by: Siva Rebbagondla +Signed-off-by: Kalle Valo +Signed-off-by: Sasha Levin +--- + drivers/net/wireless/rsi/rsi_91x_sdio.c | 18 ++++++++++++++++++ + drivers/net/wireless/rsi/rsi_91x_usb.c | 7 +++++++ + 2 files changed, 25 insertions(+) + +diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c +index 5733e440ecaff..81cc1044532d1 100644 +--- a/drivers/net/wireless/rsi/rsi_91x_sdio.c ++++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c +@@ -1129,6 +1129,12 @@ static void rsi_disconnect(struct sdio_func *pfunction) + rsi_mac80211_detach(adapter); + mdelay(10); + ++ if (IS_ENABLED(CONFIG_RSI_COEX) && adapter->priv->coex_mode > 1 && ++ adapter->priv->bt_adapter) { ++ rsi_bt_ops.detach(adapter->priv->bt_adapter); ++ adapter->priv->bt_adapter = NULL; ++ } ++ + /* Reset Chip */ + rsi_reset_chip(adapter); + +@@ -1305,6 +1311,12 @@ static int rsi_freeze(struct device *dev) + rsi_dbg(ERR_ZONE, + "##### Device can not wake up through WLAN\n"); + ++ if (IS_ENABLED(CONFIG_RSI_COEX) && common->coex_mode > 1 && ++ common->bt_adapter) { ++ rsi_bt_ops.detach(common->bt_adapter); ++ common->bt_adapter = NULL; ++ } ++ + ret = rsi_sdio_disable_interrupts(pfunction); + + if (sdev->write_fail) +@@ -1352,6 +1364,12 @@ static void rsi_shutdown(struct device *dev) + if (rsi_config_wowlan(adapter, wowlan)) + rsi_dbg(ERR_ZONE, "Failed to configure WoWLAN\n"); + ++ if (IS_ENABLED(CONFIG_RSI_COEX) && adapter->priv->coex_mode > 1 && ++ adapter->priv->bt_adapter) { ++ rsi_bt_ops.detach(adapter->priv->bt_adapter); ++ adapter->priv->bt_adapter = NULL; ++ } ++ + rsi_sdio_disable_interrupts(sdev->pfunction); + + if (sdev->write_fail) +diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c +index 90eb749e2b616..c62e7e0f82f32 100644 +--- a/drivers/net/wireless/rsi/rsi_91x_usb.c ++++ b/drivers/net/wireless/rsi/rsi_91x_usb.c +@@ -818,6 +818,13 @@ static void rsi_disconnect(struct usb_interface *pfunction) + return; + + rsi_mac80211_detach(adapter); ++ ++ if (IS_ENABLED(CONFIG_RSI_COEX) && adapter->priv->coex_mode > 1 && ++ adapter->priv->bt_adapter) { ++ rsi_bt_ops.detach(adapter->priv->bt_adapter); ++ adapter->priv->bt_adapter = NULL; ++ } ++ + rsi_reset_card(adapter); + rsi_deinit_usb_interface(adapter); + rsi_91x_deinit(adapter); +-- +2.20.1 + diff --git a/queue-4.19/rsi-fix-use-after-free-on-failed-probe-and-unbind.patch b/queue-4.19/rsi-fix-use-after-free-on-failed-probe-and-unbind.patch new file mode 100644 index 00000000000..c05b2ae348d --- /dev/null +++ b/queue-4.19/rsi-fix-use-after-free-on-failed-probe-and-unbind.patch @@ -0,0 +1,76 @@ +From 98d16c77c7b71c29805572b4a5a1a2930c814bb2 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Thu, 28 Nov 2019 18:22:00 +0100 +Subject: rsi: fix use-after-free on failed probe and unbind + +From: Johan Hovold + +[ Upstream commit e93cd35101b61e4c79149be2cfc927c4b28dc60c ] + +Make sure to stop both URBs before returning after failed probe as well +as on disconnect to avoid use-after-free in the completion handler. + +Reported-by: syzbot+b563b7f8dbe8223a51e8@syzkaller.appspotmail.com +Fixes: a4302bff28e2 ("rsi: add bluetooth rx endpoint") +Fixes: dad0d04fa7ba ("rsi: Add RS9113 wireless driver") +Cc: stable # 3.15 +Cc: Siva Rebbagondla +Cc: Prameela Rani Garnepudi +Cc: Amitkumar Karwar +Cc: Fariya Fatima +Signed-off-by: Johan Hovold +Signed-off-by: Kalle Valo +Signed-off-by: Sasha Levin +--- + drivers/net/wireless/rsi/rsi_91x_usb.c | 18 +++++++++++++++++- + 1 file changed, 17 insertions(+), 1 deletion(-) + +diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c +index c62e7e0f82f32..54106646445a9 100644 +--- a/drivers/net/wireless/rsi/rsi_91x_usb.c ++++ b/drivers/net/wireless/rsi/rsi_91x_usb.c +@@ -291,6 +291,15 @@ static void rsi_rx_done_handler(struct urb *urb) + dev_kfree_skb(rx_cb->rx_skb); + } + ++static void rsi_rx_urb_kill(struct rsi_hw *adapter, u8 ep_num) ++{ ++ struct rsi_91x_usbdev *dev = (struct rsi_91x_usbdev *)adapter->rsi_dev; ++ struct rx_usb_ctrl_block *rx_cb = &dev->rx_cb[ep_num - 1]; ++ struct urb *urb = rx_cb->rx_urb; ++ ++ usb_kill_urb(urb); ++} ++ + /** + * rsi_rx_urb_submit() - This function submits the given URB to the USB stack. + * @adapter: Pointer to the adapter structure. +@@ -791,10 +800,13 @@ static int rsi_probe(struct usb_interface *pfunction, + if (adapter->priv->coex_mode > 1) { + status = rsi_rx_urb_submit(adapter, BT_EP, GFP_KERNEL); + if (status) +- goto err1; ++ goto err_kill_wlan_urb; + } + + return 0; ++ ++err_kill_wlan_urb: ++ rsi_rx_urb_kill(adapter, WLAN_EP); + err1: + rsi_deinit_usb_interface(adapter); + err: +@@ -825,6 +837,10 @@ static void rsi_disconnect(struct usb_interface *pfunction) + adapter->priv->bt_adapter = NULL; + } + ++ if (adapter->priv->coex_mode > 1) ++ rsi_rx_urb_kill(adapter, BT_EP); ++ rsi_rx_urb_kill(adapter, WLAN_EP); ++ + rsi_reset_card(adapter); + rsi_deinit_usb_interface(adapter); + rsi_91x_deinit(adapter); +-- +2.20.1 + diff --git a/queue-4.19/series b/queue-4.19/series index ec6c1699354..0920d05d400 100644 --- a/queue-4.19/series +++ b/queue-4.19/series @@ -1 +1,7 @@ vfs-fix-do_last-regression.patch +x86-resctrl-fix-use-after-free-when-deleting-resourc.patch +x86-resctrl-fix-use-after-free-due-to-inaccurate-ref.patch +x86-resctrl-fix-a-deadlock-due-to-inaccurate-referen.patch +crypto-pcrypt-fix-user-after-free-on-module-unload.patch +rsi-add-hci-detach-for-hibernation-and-poweroff.patch +rsi-fix-use-after-free-on-failed-probe-and-unbind.patch diff --git a/queue-4.19/x86-resctrl-fix-a-deadlock-due-to-inaccurate-referen.patch b/queue-4.19/x86-resctrl-fix-a-deadlock-due-to-inaccurate-referen.patch new file mode 100644 index 00000000000..a3f1b954c7c --- /dev/null +++ b/queue-4.19/x86-resctrl-fix-a-deadlock-due-to-inaccurate-referen.patch @@ -0,0 +1,230 @@ +From aaccdeffadc697ccb68242cb0bda35f49b5d6a42 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Sun, 2 Feb 2020 06:04:22 +0800 +Subject: x86/resctrl: Fix a deadlock due to inaccurate reference + +From: Xiaochen Shen + +commit 334b0f4e9b1b4a1d475f803419d202f6c5e4d18e upstream. + +There is a race condition which results in a deadlock when rmdir and +mkdir execute concurrently: + +$ ls /sys/fs/resctrl/c1/mon_groups/m1/ +cpus cpus_list mon_data tasks + +Thread 1: rmdir /sys/fs/resctrl/c1 +Thread 2: mkdir /sys/fs/resctrl/c1/mon_groups/m1 + +3 locks held by mkdir/48649: + #0: (sb_writers#17){.+.+}, at: [] mnt_want_write+0x20/0x50 + #1: (&type->i_mutex_dir_key#8/1){+.+.}, at: [] filename_create+0x7b/0x170 + #2: (rdtgroup_mutex){+.+.}, at: [] rdtgroup_kn_lock_live+0x3d/0x70 + +4 locks held by rmdir/48652: + #0: (sb_writers#17){.+.+}, at: [] mnt_want_write+0x20/0x50 + #1: (&type->i_mutex_dir_key#8/1){+.+.}, at: [] do_rmdir+0x13f/0x1e0 + #2: (&type->i_mutex_dir_key#8){++++}, at: [] vfs_rmdir+0x4d/0x120 + #3: (rdtgroup_mutex){+.+.}, at: [] rdtgroup_kn_lock_live+0x3d/0x70 + +Thread 1 is deleting control group "c1". Holding rdtgroup_mutex, +kernfs_remove() removes all kernfs nodes under directory "c1" +recursively, then waits for sub kernfs node "mon_groups" to drop active +reference. + +Thread 2 is trying to create a subdirectory "m1" in the "mon_groups" +directory. The wrapper kernfs_iop_mkdir() takes an active reference to +the "mon_groups" directory but the code drops the active reference to +the parent directory "c1" instead. + +As a result, Thread 1 is blocked on waiting for active reference to drop +and never release rdtgroup_mutex, while Thread 2 is also blocked on +trying to get rdtgroup_mutex. + +Thread 1 (rdtgroup_rmdir) Thread 2 (rdtgroup_mkdir) +(rmdir /sys/fs/resctrl/c1) (mkdir /sys/fs/resctrl/c1/mon_groups/m1) +------------------------- ------------------------- + kernfs_iop_mkdir + /* + * kn: "m1", parent_kn: "mon_groups", + * prgrp_kn: parent_kn->parent: "c1", + * + * "mon_groups", parent_kn->active++: 1 + */ + kernfs_get_active(parent_kn) +kernfs_iop_rmdir + /* "c1", kn->active++ */ + kernfs_get_active(kn) + + rdtgroup_kn_lock_live + atomic_inc(&rdtgrp->waitcount) + /* "c1", kn->active-- */ + kernfs_break_active_protection(kn) + mutex_lock + + rdtgroup_rmdir_ctrl + free_all_child_rdtgrp + sentry->flags = RDT_DELETED + + rdtgroup_ctrl_remove + rdtgrp->flags = RDT_DELETED + kernfs_get(kn) + kernfs_remove(rdtgrp->kn) + __kernfs_remove + /* "mon_groups", sub_kn */ + atomic_add(KN_DEACTIVATED_BIAS, &sub_kn->active) + kernfs_drain(sub_kn) + /* + * sub_kn->active == KN_DEACTIVATED_BIAS + 1, + * waiting on sub_kn->active to drop, but it + * never drops in Thread 2 which is blocked + * on getting rdtgroup_mutex. + */ +Thread 1 hangs here ----> + wait_event(sub_kn->active == KN_DEACTIVATED_BIAS) + ... + rdtgroup_mkdir + rdtgroup_mkdir_mon(parent_kn, prgrp_kn) + mkdir_rdt_prepare(parent_kn, prgrp_kn) + rdtgroup_kn_lock_live(prgrp_kn) + atomic_inc(&rdtgrp->waitcount) + /* + * "c1", prgrp_kn->active-- + * + * The active reference on "c1" is + * dropped, but not matching the + * actual active reference taken + * on "mon_groups", thus causing + * Thread 1 to wait forever while + * holding rdtgroup_mutex. + */ + kernfs_break_active_protection( + prgrp_kn) + /* + * Trying to get rdtgroup_mutex + * which is held by Thread 1. + */ +Thread 2 hangs here ----> mutex_lock + ... + +The problem is that the creation of a subdirectory in the "mon_groups" +directory incorrectly releases the active protection of its parent +directory instead of itself before it starts waiting for rdtgroup_mutex. +This is triggered by the rdtgroup_mkdir() flow calling +rdtgroup_kn_lock_live()/rdtgroup_kn_unlock() with kernfs node of the +parent control group ("c1") as argument. It should be called with kernfs +node "mon_groups" instead. What is currently missing is that the +kn->priv of "mon_groups" is NULL instead of pointing to the rdtgrp. + +Fix it by pointing kn->priv to rdtgrp when "mon_groups" is created. Then +it could be passed to rdtgroup_kn_lock_live()/rdtgroup_kn_unlock() +instead. And then it operates on the same rdtgroup structure but handles +the active reference of kernfs node "mon_groups" to prevent deadlock. +The same changes are also made to the "mon_data" directories. + +This results in some unused function parameters that will be cleaned up +in follow-up patch as the focus here is on the fix only in support of +backporting efforts. + +Backporting notes: + +Since upstream commit fa7d949337cc ("x86/resctrl: Rename and move rdt +files to a separate directory"), the file +arch/x86/kernel/cpu/intel_rdt_rdtgroup.c has been renamed and moved to +arch/x86/kernel/cpu/resctrl/rdtgroup.c. +Apply the change against file arch/x86/kernel/cpu/intel_rdt_rdtgroup.c +for older stable trees. + +Fixes: c7d9aac61311 ("x86/intel_rdt/cqm: Add mkdir support for RDT monitoring") +Suggested-by: Reinette Chatre +Signed-off-by: Xiaochen Shen +Signed-off-by: Borislav Petkov +Reviewed-by: Reinette Chatre +Reviewed-by: Tony Luck +Acked-by: Thomas Gleixner +Cc: stable@vger.kernel.org +Link: https://lkml.kernel.org/r/1578500886-21771-4-git-send-email-xiaochen.shen@intel.com +Signed-off-by: Sasha Levin +--- + arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 16 ++++++++-------- + 1 file changed, 8 insertions(+), 8 deletions(-) + +diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c +index 77770caeea242..11c5accfa4db5 100644 +--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c ++++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c +@@ -2005,7 +2005,7 @@ static struct dentry *rdt_mount(struct file_system_type *fs_type, + + if (rdt_mon_capable) { + ret = mongroup_create_dir(rdtgroup_default.kn, +- NULL, "mon_groups", ++ &rdtgroup_default, "mon_groups", + &kn_mongrp); + if (ret) { + dentry = ERR_PTR(ret); +@@ -2415,7 +2415,7 @@ static int mkdir_mondata_all(struct kernfs_node *parent_kn, + /* + * Create the mon_data directory first. + */ +- ret = mongroup_create_dir(parent_kn, NULL, "mon_data", &kn); ++ ret = mongroup_create_dir(parent_kn, prgrp, "mon_data", &kn); + if (ret) + return ret; + +@@ -2568,7 +2568,7 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn, + uint files = 0; + int ret; + +- prdtgrp = rdtgroup_kn_lock_live(prgrp_kn); ++ prdtgrp = rdtgroup_kn_lock_live(parent_kn); + rdt_last_cmd_clear(); + if (!prdtgrp) { + ret = -ENODEV; +@@ -2643,7 +2643,7 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn, + kernfs_activate(kn); + + /* +- * The caller unlocks the prgrp_kn upon success. ++ * The caller unlocks the parent_kn upon success. + */ + return 0; + +@@ -2654,7 +2654,7 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn, + out_free_rgrp: + kfree(rdtgrp); + out_unlock: +- rdtgroup_kn_unlock(prgrp_kn); ++ rdtgroup_kn_unlock(parent_kn); + return ret; + } + +@@ -2692,7 +2692,7 @@ static int rdtgroup_mkdir_mon(struct kernfs_node *parent_kn, + */ + list_add_tail(&rdtgrp->mon.crdtgrp_list, &prgrp->mon.crdtgrp_list); + +- rdtgroup_kn_unlock(prgrp_kn); ++ rdtgroup_kn_unlock(parent_kn); + return ret; + } + +@@ -2735,7 +2735,7 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn, + * Create an empty mon_groups directory to hold the subset + * of tasks and cpus to monitor. + */ +- ret = mongroup_create_dir(kn, NULL, "mon_groups", NULL); ++ ret = mongroup_create_dir(kn, rdtgrp, "mon_groups", NULL); + if (ret) { + rdt_last_cmd_puts("kernfs subdir error\n"); + goto out_del_list; +@@ -2751,7 +2751,7 @@ static int rdtgroup_mkdir_ctrl_mon(struct kernfs_node *parent_kn, + out_common_fail: + mkdir_rdt_prepare_clean(rdtgrp); + out_unlock: +- rdtgroup_kn_unlock(prgrp_kn); ++ rdtgroup_kn_unlock(parent_kn); + return ret; + } + +-- +2.20.1 + diff --git a/queue-4.19/x86-resctrl-fix-use-after-free-due-to-inaccurate-ref.patch b/queue-4.19/x86-resctrl-fix-use-after-free-due-to-inaccurate-ref.patch new file mode 100644 index 00000000000..a97cafceff3 --- /dev/null +++ b/queue-4.19/x86-resctrl-fix-use-after-free-due-to-inaccurate-ref.patch @@ -0,0 +1,137 @@ +From 97e78cdfdaec1efd2939046eb26977363ba907ac Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Sun, 2 Feb 2020 06:03:43 +0800 +Subject: x86/resctrl: Fix use-after-free due to inaccurate refcount of + rdtgroup + +From: Xiaochen Shen + +commit 074fadee59ee7a9d2b216e9854bd4efb5dad679f upstream. + +There is a race condition in the following scenario which results in an +use-after-free issue when reading a monitoring file and deleting the +parent ctrl_mon group concurrently: + +Thread 1 calls atomic_inc() to take refcount of rdtgrp and then calls +kernfs_break_active_protection() to drop the active reference of kernfs +node in rdtgroup_kn_lock_live(). + +In Thread 2, kernfs_remove() is a blocking routine. It waits on all sub +kernfs nodes to drop the active reference when removing all subtree +kernfs nodes recursively. Thread 2 could block on kernfs_remove() until +Thread 1 calls kernfs_break_active_protection(). Only after +kernfs_remove() completes the refcount of rdtgrp could be trusted. + +Before Thread 1 calls atomic_inc() and kernfs_break_active_protection(), +Thread 2 could call kfree() when the refcount of rdtgrp (sentry) is 0 +instead of 1 due to the race. + +In Thread 1, in rdtgroup_kn_unlock(), referring to earlier rdtgrp memory +(rdtgrp->waitcount) which was already freed in Thread 2 results in +use-after-free issue. + +Thread 1 (rdtgroup_mondata_show) Thread 2 (rdtgroup_rmdir) +-------------------------------- ------------------------- +rdtgroup_kn_lock_live + /* + * kn active protection until + * kernfs_break_active_protection(kn) + */ + rdtgrp = kernfs_to_rdtgroup(kn) + rdtgroup_kn_lock_live + atomic_inc(&rdtgrp->waitcount) + mutex_lock + rdtgroup_rmdir_ctrl + free_all_child_rdtgrp + /* + * sentry->waitcount should be 1 + * but is 0 now due to the race. + */ + kfree(sentry)*[1] + /* + * Only after kernfs_remove() + * completes, the refcount of + * rdtgrp could be trusted. + */ + atomic_inc(&rdtgrp->waitcount) + /* kn->active-- */ + kernfs_break_active_protection(kn) + rdtgroup_ctrl_remove + rdtgrp->flags = RDT_DELETED + /* + * Blocking routine, wait for + * all sub kernfs nodes to drop + * active reference in + * kernfs_break_active_protection. + */ + kernfs_remove(rdtgrp->kn) + rdtgroup_kn_unlock + mutex_unlock + atomic_dec_and_test( + &rdtgrp->waitcount) + && (flags & RDT_DELETED) + kernfs_unbreak_active_protection(kn) + kfree(rdtgrp) + mutex_lock +mon_event_read +rdtgroup_kn_unlock + mutex_unlock + /* + * Use-after-free: refer to earlier rdtgrp + * memory which was freed in [1]. + */ + atomic_dec_and_test(&rdtgrp->waitcount) + && (flags & RDT_DELETED) + /* kn->active++ */ + kernfs_unbreak_active_protection(kn) + kfree(rdtgrp) + +Fix it by moving free_all_child_rdtgrp() to after kernfs_remove() in +rdtgroup_rmdir_ctrl() to ensure it has the accurate refcount of rdtgrp. + +Backporting notes: + +Since upstream commit fa7d949337cc ("x86/resctrl: Rename and move rdt +files to a separate directory"), the file +arch/x86/kernel/cpu/intel_rdt_rdtgroup.c has been renamed and moved to +arch/x86/kernel/cpu/resctrl/rdtgroup.c. +Apply the change against file arch/x86/kernel/cpu/intel_rdt_rdtgroup.c +for older stable trees. + +Fixes: f3cbeacaa06e ("x86/intel_rdt/cqm: Add rmdir support") +Suggested-by: Reinette Chatre +Signed-off-by: Xiaochen Shen +Signed-off-by: Borislav Petkov +Reviewed-by: Reinette Chatre +Reviewed-by: Tony Luck +Acked-by: Thomas Gleixner +Cc: stable@vger.kernel.org +Link: https://lkml.kernel.org/r/1578500886-21771-3-git-send-email-xiaochen.shen@intel.com +Signed-off-by: Sasha Levin +--- + arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c +index db22ba0bf9167..77770caeea242 100644 +--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c ++++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c +@@ -2877,13 +2877,13 @@ static int rdtgroup_rmdir_ctrl(struct kernfs_node *kn, struct rdtgroup *rdtgrp, + closid_free(rdtgrp->closid); + free_rmid(rdtgrp->mon.rmid); + ++ rdtgroup_ctrl_remove(kn, rdtgrp); ++ + /* + * Free all the child monitor group rmids. + */ + free_all_child_rdtgrp(rdtgrp); + +- rdtgroup_ctrl_remove(kn, rdtgrp); +- + return 0; + } + +-- +2.20.1 + diff --git a/queue-4.19/x86-resctrl-fix-use-after-free-when-deleting-resourc.patch b/queue-4.19/x86-resctrl-fix-use-after-free-when-deleting-resourc.patch new file mode 100644 index 00000000000..31dfa3e653d --- /dev/null +++ b/queue-4.19/x86-resctrl-fix-use-after-free-when-deleting-resourc.patch @@ -0,0 +1,234 @@ +From 89358c1b24f2a9f6b98d7f79f7db77c0e3538d90 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Sun, 2 Feb 2020 06:02:17 +0800 +Subject: x86/resctrl: Fix use-after-free when deleting resource groups + +From: Xiaochen Shen + +commit b8511ccc75c033f6d54188ea4df7bf1e85778740 upstream. + +A resource group (rdtgrp) contains a reference count (rdtgrp->waitcount) +that indicates how many waiters expect this rdtgrp to exist. Waiters +could be waiting on rdtgroup_mutex or some work sitting on a task's +workqueue for when the task returns from kernel mode or exits. + +The deletion of a rdtgrp is intended to have two phases: + + (1) while holding rdtgroup_mutex the necessary cleanup is done and + rdtgrp->flags is set to RDT_DELETED, + + (2) after releasing the rdtgroup_mutex, the rdtgrp structure is freed + only if there are no waiters and its flag is set to RDT_DELETED. Upon + gaining access to rdtgroup_mutex or rdtgrp, a waiter is required to check + for the RDT_DELETED flag. + +When unmounting the resctrl file system or deleting ctrl_mon groups, +all of the subdirectories are removed and the data structure of rdtgrp +is forcibly freed without checking rdtgrp->waitcount. If at this point +there was a waiter on rdtgrp then a use-after-free issue occurs when the +waiter starts running and accesses the rdtgrp structure it was waiting +on. + +See kfree() calls in [1], [2] and [3] in these two call paths in +following scenarios: +(1) rdt_kill_sb() -> rmdir_all_sub() -> free_all_child_rdtgrp() +(2) rdtgroup_rmdir() -> rdtgroup_rmdir_ctrl() -> free_all_child_rdtgrp() + +There are several scenarios that result in use-after-free issue in +following: + +Scenario 1: +----------- +In Thread 1, rdtgroup_tasks_write() adds a task_work callback +move_myself(). If move_myself() is scheduled to execute after Thread 2 +rdt_kill_sb() is finished, referring to earlier rdtgrp memory +(rdtgrp->waitcount) which was already freed in Thread 2 results in +use-after-free issue. + +Thread 1 (rdtgroup_tasks_write) Thread 2 (rdt_kill_sb) +------------------------------- ---------------------- +rdtgroup_kn_lock_live + atomic_inc(&rdtgrp->waitcount) + mutex_lock +rdtgroup_move_task + __rdtgroup_move_task + /* + * Take an extra refcount, so rdtgrp cannot be freed + * before the call back move_myself has been invoked + */ + atomic_inc(&rdtgrp->waitcount) + /* Callback move_myself will be scheduled for later */ + task_work_add(move_myself) +rdtgroup_kn_unlock + mutex_unlock + atomic_dec_and_test(&rdtgrp->waitcount) + && (flags & RDT_DELETED) + mutex_lock + rmdir_all_sub + /* + * sentry and rdtgrp are freed + * without checking refcount + */ + free_all_child_rdtgrp + kfree(sentry)*[1] + kfree(rdtgrp)*[2] + mutex_unlock +/* + * Callback is scheduled to execute + * after rdt_kill_sb is finished + */ +move_myself + /* + * Use-after-free: refer to earlier rdtgrp + * memory which was freed in [1] or [2]. + */ + atomic_dec_and_test(&rdtgrp->waitcount) + && (flags & RDT_DELETED) + kfree(rdtgrp) + +Scenario 2: +----------- +In Thread 1, rdtgroup_tasks_write() adds a task_work callback +move_myself(). If move_myself() is scheduled to execute after Thread 2 +rdtgroup_rmdir() is finished, referring to earlier rdtgrp memory +(rdtgrp->waitcount) which was already freed in Thread 2 results in +use-after-free issue. + +Thread 1 (rdtgroup_tasks_write) Thread 2 (rdtgroup_rmdir) +------------------------------- ------------------------- +rdtgroup_kn_lock_live + atomic_inc(&rdtgrp->waitcount) + mutex_lock +rdtgroup_move_task + __rdtgroup_move_task + /* + * Take an extra refcount, so rdtgrp cannot be freed + * before the call back move_myself has been invoked + */ + atomic_inc(&rdtgrp->waitcount) + /* Callback move_myself will be scheduled for later */ + task_work_add(move_myself) +rdtgroup_kn_unlock + mutex_unlock + atomic_dec_and_test(&rdtgrp->waitcount) + && (flags & RDT_DELETED) + rdtgroup_kn_lock_live + atomic_inc(&rdtgrp->waitcount) + mutex_lock + rdtgroup_rmdir_ctrl + free_all_child_rdtgrp + /* + * sentry is freed without + * checking refcount + */ + kfree(sentry)*[3] + rdtgroup_ctrl_remove + rdtgrp->flags = RDT_DELETED + rdtgroup_kn_unlock + mutex_unlock + atomic_dec_and_test( + &rdtgrp->waitcount) + && (flags & RDT_DELETED) + kfree(rdtgrp) +/* + * Callback is scheduled to execute + * after rdt_kill_sb is finished + */ +move_myself + /* + * Use-after-free: refer to earlier rdtgrp + * memory which was freed in [3]. + */ + atomic_dec_and_test(&rdtgrp->waitcount) + && (flags & RDT_DELETED) + kfree(rdtgrp) + +If CONFIG_DEBUG_SLAB=y, Slab corruption on kmalloc-2k can be observed +like following. Note that "0x6b" is POISON_FREE after kfree(). The +corrupted bits "0x6a", "0x64" at offset 0x424 correspond to +waitcount member of struct rdtgroup which was freed: + + Slab corruption (Not tainted): kmalloc-2k start=ffff9504c5b0d000, len=2048 + 420: 6b 6b 6b 6b 6a 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkjkkkkkkkkkkk + Single bit error detected. Probably bad RAM. + Run memtest86+ or a similar memory test tool. + Next obj: start=ffff9504c5b0d800, len=2048 + 000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk + 010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk + + Slab corruption (Not tainted): kmalloc-2k start=ffff9504c58ab800, len=2048 + 420: 6b 6b 6b 6b 64 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkdkkkkkkkkkkk + Prev obj: start=ffff9504c58ab000, len=2048 + 000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk + 010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk + +Fix this by taking reference count (waitcount) of rdtgrp into account in +the two call paths that currently do not do so. Instead of always +freeing the resource group it will only be freed if there are no waiters +on it. If there are waiters, the resource group will have its flags set +to RDT_DELETED. + +It will be left to the waiter to free the resource group when it starts +running and finding that it was the last waiter and the resource group +has been removed (rdtgrp->flags & RDT_DELETED) since. (1) rdt_kill_sb() +-> rmdir_all_sub() -> free_all_child_rdtgrp() (2) rdtgroup_rmdir() -> +rdtgroup_rmdir_ctrl() -> free_all_child_rdtgrp() + +Backporting notes: + +Since upstream commit fa7d949337cc ("x86/resctrl: Rename and move rdt +files to a separate directory"), the file +arch/x86/kernel/cpu/intel_rdt_rdtgroup.c has been renamed and moved to +arch/x86/kernel/cpu/resctrl/rdtgroup.c. + +Apply the change against file arch/x86/kernel/cpu/intel_rdt_rdtgroup.c +in older stable trees. + +Fixes: f3cbeacaa06e ("x86/intel_rdt/cqm: Add rmdir support") +Fixes: 60cf5e101fd4 ("x86/intel_rdt: Add mkdir to resctrl file system") +Suggested-by: Reinette Chatre +Signed-off-by: Xiaochen Shen +Signed-off-by: Borislav Petkov +Reviewed-by: Reinette Chatre +Reviewed-by: Tony Luck +Acked-by: Thomas Gleixner +Cc: stable@vger.kernel.org +Link: https://lkml.kernel.org/r/1578500886-21771-2-git-send-email-xiaochen.shen@intel.com +Signed-off-by: Sasha Levin +--- + arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 12 ++++++++++-- + 1 file changed, 10 insertions(+), 2 deletions(-) + +diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c +index 841a0246eb897..db22ba0bf9167 100644 +--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c ++++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c +@@ -2167,7 +2167,11 @@ static void free_all_child_rdtgrp(struct rdtgroup *rdtgrp) + list_for_each_entry_safe(sentry, stmp, head, mon.crdtgrp_list) { + free_rmid(sentry->mon.rmid); + list_del(&sentry->mon.crdtgrp_list); +- kfree(sentry); ++ ++ if (atomic_read(&sentry->waitcount) != 0) ++ sentry->flags = RDT_DELETED; ++ else ++ kfree(sentry); + } + } + +@@ -2205,7 +2209,11 @@ static void rmdir_all_sub(void) + + kernfs_remove(rdtgrp->kn); + list_del(&rdtgrp->rdtgroup_list); +- kfree(rdtgrp); ++ ++ if (atomic_read(&rdtgrp->waitcount) != 0) ++ rdtgrp->flags = RDT_DELETED; ++ else ++ kfree(rdtgrp); + } + /* Notify online CPUs to update per cpu storage and PQR_ASSOC MSR */ + update_closid_rmid(cpu_online_mask, &rdtgroup_default); +-- +2.20.1 + -- 2.47.3