On 5/30/26 9:50 AM, John Groves wrote:
> From: John Groves <[email protected]>
>
> Clear dev_dax->pgmap on probe failure for dynamic devices. After the dynamic
> path sets dev_dax->pgmap, if a later probe step fails, devres frees the
> devm_kzalloc'd pgmap but leaves dev_dax->pgmap dangling. Subsequent probe
> attempts would hit the "dynamic-dax with pre-populated page map" check and
> fail
> permanently. Use a goto cleanup to NULL dev_dax->pgmap on error.
>
> Fixes: d5406bd458b0a ("dax: add fsdev.c driver for fs-dax on character dax")
> Signed-off-by: John Groves <[email protected]>
> ---
> drivers/dax/fsdev.c | 32 +++++++++++++++++++++++---------
> 1 file changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
> index dbd722ed7ab05..42aac7e952516 100644
> --- a/drivers/dax/fsdev.c
> +++ b/drivers/dax/fsdev.c
> @@ -223,6 +223,7 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
> {
> struct dax_device *dax_dev = dev_dax->dax_dev;
> struct device *dev = &dev_dax->dev;
> + bool pgmap_allocated = false;
> struct dev_pagemap *pgmap;
> struct inode *inode;
> u64 data_offset = 0;
> @@ -253,6 +254,7 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
>
> pgmap->nr_range = dev_dax->nr_range;
> dev_dax->pgmap = pgmap;
> + pgmap_allocated = true;
>
> for (i = 0; i < dev_dax->nr_range; i++) {
> struct range *range = &dev_dax->ranges[i].range;
> @@ -268,7 +270,8 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
> range_len(range), dev_name(dev))) {
> dev_warn(dev, "mapping%d: %#llx-%#llx could not reserve
> range\n",
> i, range->start, range->end);
> - return -EBUSY;
> + rc = -EBUSY;
> + goto err_pgmap;
> }
> }
>
> @@ -288,8 +291,10 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
> pgmap->owner = dev_dax;
>
> addr = devm_memremap_pages(dev, pgmap);
> - if (IS_ERR(addr))
> - return PTR_ERR(addr);
> + if (IS_ERR(addr)) {
> + rc = PTR_ERR(addr);
> + goto err_pgmap;
> + }
>
> /*
> * Clear any stale compound folio state left over from a previous
> @@ -301,7 +306,7 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
> rc = devm_add_action_or_reset(dev, fsdev_clear_folio_state_action,
> dev_dax);
> if (rc)
> - return rc;
> + goto err_pgmap;
>
> /* Detect whether the data is at a non-zero offset into the memory */
> if (pgmap->range.start != dev_dax->ranges[0].range.start) {
> @@ -323,23 +328,32 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
> cdev_set_parent(cdev, &dev->kobj);
> rc = cdev_add(cdev, dev->devt, 1);
> if (rc)
> - return rc;
> + goto err_pgmap;
>
> rc = devm_add_action_or_reset(dev, fsdev_cdev_del, cdev);
> if (rc)
> - return rc;
> + goto err_pgmap;
>
> /* Set the dax operations for fs-dax access path */
> rc = dax_set_ops(dax_dev, &dev_dax_ops);
> if (rc)
> - return rc;
> + goto err_pgmap;
>
> rc = devm_add_action_or_reset(dev, fsdev_clear_ops, dev_dax);
> if (rc)
> - return rc;
> + goto err_pgmap;
>
> run_dax(dax_dev);
> - return devm_add_action_or_reset(dev, fsdev_kill, dev_dax);
> + rc = devm_add_action_or_reset(dev, fsdev_kill, dev_dax);
> + if (rc)
> + goto err_pgmap;
> +
> + return 0;
> +
> +err_pgmap:
> + if (pgmap_allocated)
> + dev_dax->pgmap = NULL;
> + return rc;
> }
>
> static struct dax_device_driver fsdev_dax_driver = {
How about something like this to ditch the gotos?
---
diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
index dbd722ed7ab0..cb309847e685 100644
--- a/drivers/dax/fsdev.c
+++ b/drivers/dax/fsdev.c
@@ -219,6 +219,41 @@ static const struct file_operations fsdev_fops = {
.release = fsdev_release,
};
+static struct dev_pagemap *fsdev_acquire_pgmap(struct dev_dax *dev_dax)
+{
+ struct device *dev = &dev_dax->dev;
+ struct dev_pagemap *pgmap;
+ size_t pgmap_size;
+
+ if (static_dev_dax(dev_dax)) {
+ if (dev_dax->nr_range > 1) {
+ dev_warn(dev,
+ "static vs multi-range device conflict\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ pgmap = dev_dax->pgmap;
+ pgmap->vmemmap_shift = 0;
+ return pgmap;
+ }
+
+ if (dev_dax->pgmap) {
+ dev_warn(dev, "dynamic-dax with pre-populated page map\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ pgmap_size = struct_size(pgmap, ranges, dev_dax->nr_range - 1);
+ pgmap = devm_kzalloc(dev, pgmap_size, GFP_KERNEL);
+ if (!pgmap)
+ return ERR_PTR(-ENOMEM);
+
+ pgmap->nr_range = dev_dax->nr_range;
+ for (int i = 0; i < dev_dax->nr_range; i++)
+ pgmap->ranges[i] = dev_dax->ranges[i].range;
+
+ return pgmap;
+}
+
static int fsdev_dax_probe(struct dev_dax *dev_dax)
{
struct dax_device *dax_dev = dev_dax->dax_dev;
@@ -230,36 +265,9 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
void *addr;
int rc, i;
- if (static_dev_dax(dev_dax)) {
- if (dev_dax->nr_range > 1) {
- dev_warn(dev, "static pgmap / multi-range device
conflict\n");
- return -EINVAL;
- }
-
- pgmap = dev_dax->pgmap;
- pgmap->vmemmap_shift = 0;
- } else {
- size_t pgmap_size;
-
- if (dev_dax->pgmap) {
- dev_warn(dev, "dynamic-dax with pre-populated page
map\n");
- return -EINVAL;
- }
-
- pgmap_size = struct_size(pgmap, ranges, dev_dax->nr_range - 1);
- pgmap = devm_kzalloc(dev, pgmap_size, GFP_KERNEL);
- if (!pgmap)
- return -ENOMEM;
-
- pgmap->nr_range = dev_dax->nr_range;
- dev_dax->pgmap = pgmap;
-
- for (i = 0; i < dev_dax->nr_range; i++) {
- struct range *range = &dev_dax->ranges[i].range;
-
- pgmap->ranges[i] = *range;
- }
- }
+ pgmap = fsdev_acquire_pgmap(dev_dax);
+ if (IS_ERR(pgmap))
+ return PTR_ERR(pgmap);
for (i = 0; i < dev_dax->nr_range; i++) {
struct range *range = &dev_dax->ranges[i].range;
@@ -306,7 +314,7 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
/* Detect whether the data is at a non-zero offset into the memory */
if (pgmap->range.start != dev_dax->ranges[0].range.start) {
u64 phys = dev_dax->ranges[0].range.start;
- u64 pgmap_phys = dev_dax->pgmap[0].range.start;
+ u64 pgmap_phys = pgmap[0].range.start;
if (!WARN_ON(pgmap_phys > phys))
data_offset = phys - pgmap_phys;
@@ -339,7 +347,12 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
return rc;
run_dax(dax_dev);
- return devm_add_action_or_reset(dev, fsdev_kill, dev_dax);
+ rc = devm_add_action_or_reset(dev, fsdev_kill, dev_dax);
+ if (rc)
+ return rc;
+
+ dev_dax->pgmap = pgmap;
+ return 0;
}
static struct dax_device_driver fsdev_dax_driver = {
