From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linuxfoundation.org ([140.211.169.12]:33874 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754909AbdERHp1 (ORCPT ); Thu, 18 May 2017 03:45:27 -0400 Subject: Patch "device-dax: fix sysfs attribute deadlock" has been added to the 4.11-stable tree To: dan.j.williams@intel.com, gregkh@linuxfoundation.org, yizhan@redhat.com Cc: , From: Date: Thu, 18 May 2017 09:45:15 +0200 Message-ID: <149509351572246@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ANSI_X3.4-1968 Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: This is a note to let you know that I've just added the patch titled device-dax: fix sysfs attribute deadlock to the 4.11-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: device-dax-fix-sysfs-attribute-deadlock.patch and it can be found in the queue-4.11 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let know about it. >>From 565851c972b50612f3a4542e26879ffb3e906fc2 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sun, 30 Apr 2017 06:57:01 -0700 Subject: device-dax: fix sysfs attribute deadlock From: Dan Williams commit 565851c972b50612f3a4542e26879ffb3e906fc2 upstream. Usage of device_lock() for dax_region attributes is unnecessary and deadlock prone. It's unnecessary because the order of registration / un-registration guarantees that drvdata is always valid. It's deadlock prone because it sets up this situation: ndctl D 0 2170 2082 0x00000000 Call Trace: __schedule+0x31f/0x980 schedule+0x3d/0x90 schedule_preempt_disabled+0x15/0x20 __mutex_lock+0x402/0x980 ? __mutex_lock+0x158/0x980 ? align_show+0x2b/0x80 [dax] ? kernfs_seq_start+0x2f/0x90 mutex_lock_nested+0x1b/0x20 align_show+0x2b/0x80 [dax] dev_attr_show+0x20/0x50 ndctl D 0 2186 2079 0x00000000 Call Trace: __schedule+0x31f/0x980 schedule+0x3d/0x90 __kernfs_remove+0x1f6/0x340 ? kernfs_remove_by_name_ns+0x45/0xa0 ? remove_wait_queue+0x70/0x70 kernfs_remove_by_name_ns+0x45/0xa0 remove_files.isra.1+0x35/0x70 sysfs_remove_group+0x44/0x90 sysfs_remove_groups+0x2e/0x50 dax_region_unregister+0x25/0x40 [dax] devm_action_release+0xf/0x20 release_nodes+0x16d/0x2b0 devres_release_all+0x3c/0x60 device_release_driver_internal+0x17d/0x220 device_release_driver+0x12/0x20 unbind_store+0x112/0x160 ndctl/2170 is trying to acquire the device_lock() to read an attribute, and ndctl/2186 is holding the device_lock() while trying to drain all active attribute readers. Thanks to Yi Zhang for the reproduction script. Fixes: d7fe1a67f658 ("dax: add region 'id', 'size', and 'align' attributes") Reported-by: Yi Zhang Signed-off-by: Dan Williams Signed-off-by: Greg Kroah-Hartman --- drivers/dax/dax.c | 40 ++++++++++++---------------------------- 1 file changed, 12 insertions(+), 28 deletions(-) --- a/drivers/dax/dax.c +++ b/drivers/dax/dax.c @@ -77,36 +77,27 @@ struct dax_dev { struct resource res[0]; }; +/* + * Rely on the fact that drvdata is set before the attributes are + * registered, and that the attributes are unregistered before drvdata + * is cleared to assume that drvdata is always valid. + */ static ssize_t id_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct dax_region *dax_region; - ssize_t rc = -ENXIO; + struct dax_region *dax_region = dev_get_drvdata(dev); - device_lock(dev); - dax_region = dev_get_drvdata(dev); - if (dax_region) - rc = sprintf(buf, "%d\n", dax_region->id); - device_unlock(dev); - - return rc; + return sprintf(buf, "%d\n", dax_region->id); } static DEVICE_ATTR_RO(id); static ssize_t region_size_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct dax_region *dax_region; - ssize_t rc = -ENXIO; - - device_lock(dev); - dax_region = dev_get_drvdata(dev); - if (dax_region) - rc = sprintf(buf, "%llu\n", (unsigned long long) - resource_size(&dax_region->res)); - device_unlock(dev); + struct dax_region *dax_region = dev_get_drvdata(dev); - return rc; + return sprintf(buf, "%llu\n", (unsigned long long) + resource_size(&dax_region->res)); } static struct device_attribute dev_attr_region_size = __ATTR(size, 0444, region_size_show, NULL); @@ -114,16 +105,9 @@ static struct device_attribute dev_attr_ static ssize_t align_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct dax_region *dax_region; - ssize_t rc = -ENXIO; - - device_lock(dev); - dax_region = dev_get_drvdata(dev); - if (dax_region) - rc = sprintf(buf, "%u\n", dax_region->align); - device_unlock(dev); + struct dax_region *dax_region = dev_get_drvdata(dev); - return rc; + return sprintf(buf, "%u\n", dax_region->align); } static DEVICE_ATTR_RO(align); Patches currently in stable-queue which might be from dan.j.williams@intel.com are queue-4.11/ext4-return-to-starting-transaction-in-ext4_dax_huge_fault.patch queue-4.11/x86-pmem-fix-cache-flushing-for-iovec-write-8-bytes.patch queue-4.11/dax-prevent-invalidation-of-mapped-dax-entries.patch queue-4.11/dax-fix-pmd-data-corruption-when-fault-races-with-write.patch queue-4.11/device-dax-fix-cdev-leak.patch queue-4.11/device-dax-fix-sysfs-attribute-deadlock.patch queue-4.11/mm-fix-data-corruption-due-to-stale-mmap-reads.patch