[PATCH 1/1] dax: Fix stack overflow when mounting fsdax pmem device

* [PATCH 1/1] dax: Fix stack overflow when mounting fsdax pmem device
@ 2020-09-15  7:57 Adrian Huang
  2020-09-15  8:37 ` Jan Kara
  0 siblings, 1 reply; 6+ messages in thread
From: Adrian Huang @ 2020-09-15  7:57 UTC (permalink / raw)
  To: linux-nvdimm
  Cc: Adrian Huang, Coly Li, Mikulas Patocka, Alasdair Kergon,
	Mike Snitzer, Adrian Huang

From: Adrian Huang <ahuang12@lenovo.com>

When mounting fsdax pmem device, commit 6180bb446ab6 ("dax: fix
detection of dax support for non-persistent memory block devices")
introduces the stack overflow [1][2]. Here is the call path for
mounting ext4 file system:
  ext4_fill_super
    bdev_dax_supported
      __bdev_dax_supported
        dax_supported
          generic_fsdax_supported
            __generic_fsdax_supported
              bdev_dax_supported

The call path leads to the infinite calling loop, so we cannot
call bdev_dax_supported() in __generic_fsdax_supported(). The sanity
checking of the variable 'dax_dev' is moved prior to the two
bdev_dax_pgoff() checks [3][4].

To fix the issue triggered by lvm2-testsuite (the issue that the
above-mentioned commit wants to fix), this patch does not print the
"error: dax access failed" message if the physical disk does not
support DAX (dax_dev is NULL). The detail info is described as follows:

  1. The dax_dev of the dm devices (dm-0, dm-1..) is always allocated
     in alloc_dev() [drivers/md/dm.c].
  2. When calling __generic_fsdax_supported() with dm-0 device, the
     call path is shown as follows (the physical disks of dm-0 do
     not support DAX):
        dax_direct_access (valid dax_dev with dm-0)
          dax_dev->ops->direct_access
            dm_dax_direct_access
              ti->type->direct_access
                linear_dax_direct_access (assume the target is linear)
                  dax_direct_access (dax_dev is NULLL with ram0, or sdaX)
  3. The call 'dax_direct_access()' in __generic_fsdax_supported() gets
     the returned value '-EOPNOTSUPP'.
  4. However, the message 'dm-3: error: dax access failed (-5)' is still
     printed for the dm target 'error' since io_err_dax_direct_access()
     always returns the status '-EIO'. Cc' device mapper maintainers to
     see if they have concerns.

[1] https://lists.01.org/hyperkitty/list/linux-nvdimm@lists.01.org/thread/BULZHRILK7N2WS2JVISNF2QZNRQK6JU4/
[2] https://lists.01.org/hyperkitty/list/linux-nvdimm@lists.01.org/thread/OOZGFY3RNQGTGJJCH52YXCSYIDXMOPXO/
[3] https://lists.01.org/hyperkitty/list/linux-nvdimm@lists.01.org/message/SMQW2LY3QHPXOAW76RKNSCGG3QJFO7HT/
[4] https://lists.01.org/hyperkitty/list/linux-nvdimm@lists.01.org/message/7E2X6UGX5RQ2ISGYNAF66VLY5BKBFI4M/

Fixes: 6180bb446ab6 ("dax: fix detection of dax support for non-persistent memory block devices")
Cc: Coly Li <colyli@suse.de>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: John Pittman <jpittman@redhat.com>
Cc: Mikulas Patocka <mpatocka@redhat.com>
Cc: Alasdair Kergon <agk@redhat.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Adrian Huang <ahuang12@lenovo.com>
---
 drivers/dax/super.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index e5767c83ea23..fb151417ec10 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -85,6 +85,12 @@ bool __generic_fsdax_supported(struct dax_device *dax_dev,
 		return false;
 	}
 
+	if (!dax_dev) {
+		pr_debug("%s: error: dax unsupported by block device\n",
+				bdevname(bdev, buf));
+		return false;
+	}
+
 	err = bdev_dax_pgoff(bdev, start, PAGE_SIZE, &pgoff);
 	if (err) {
 		pr_info("%s: error: unaligned partition for dax\n",
@@ -100,19 +106,22 @@ bool __generic_fsdax_supported(struct dax_device *dax_dev,
 		return false;
 	}
 
-	if (!dax_dev || !bdev_dax_supported(bdev, blocksize)) {
-		pr_debug("%s: error: dax unsupported by block device\n",
-				bdevname(bdev, buf));
-		return false;
-	}
-
 	id = dax_read_lock();
 	len = dax_direct_access(dax_dev, pgoff, 1, &kaddr, &pfn);
 	len2 = dax_direct_access(dax_dev, pgoff_end, 1, &end_kaddr, &end_pfn);
 
 	if (len < 1 || len2 < 1) {
-		pr_info("%s: error: dax access failed (%ld)\n",
+		/*
+		 * Only print the real error message: do not need to print
+		 * the message for the underlying raw disk (physical disk)
+		 * that does not support DAX (dax_dev = NULL). This case
+		 * is observed when physical disks are configured by
+		 * lvm2 (device mapper).
+		 */
+		if (len != -EOPNOTSUPP && len2 != -EOPNOTSUPP) {
+			pr_info("%s: error: dax access failed (%ld)\n",
 				bdevname(bdev, buf), len < 1 ? len : len2);
+		}
 		dax_read_unlock(id);
 		return false;
 	}
-- 
2.17.1
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

^ permalink raw reply related	[flat|nested] 6+ messages in thread