Linux CXL
 help / color / mirror / Atom feed
* [PATCH] dax/device: defer publishing the dynamic pgmap
@ 2026-09-27 11:47 Jiale Yao
  2026-09-27 11:58 ` sashiko-bot
  2026-09-27 20:05 ` Andrew Morton
  0 siblings, 2 replies; 5+ messages in thread
From: Jiale Yao @ 2026-09-27 11:47 UTC (permalink / raw)
  To: Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield,
	Andrew Morton, Joao Martins, nvdimm, linux-cxl, linux-kernel
  Cc: Jiale Yao, stable

The dynamic device-dax probe path publishes its devm-allocated pgmap
before several operations that can still fail.  If one of them fails,
devres frees the pgmap while dev_dax->pgmap remains non-NULL.  A later
bind then fails the dynamic-dax invariant check and leaves the device
unusable until its region is recreated.

Defer assigning dev_dax->pgmap until the final devm action has been
installed and probe can no longer fail.  A failed probe then never
publishes the temporary pgmap.

Fixes: fc65c4eb0b2a ("device-dax: ensure dev_dax->pgmap is valid for dynamic devices")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/dax/device.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/dax/device.c b/drivers/dax/device.c
index d0c9b4e03b47..8cd8e05872c3 100644
--- a/drivers/dax/device.c
+++ b/drivers/dax/device.c
@@ -409,7 +409,6 @@ static int dev_dax_probe(struct dev_dax *dev_dax)
 			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;
@@ -450,7 +449,13 @@ static int dev_dax_probe(struct dev_dax *dev_dax)
 		return rc;
 
 	run_dax(dax_dev);
-	return devm_add_action_or_reset(dev, dev_dax_kill, dev_dax);
+	rc = devm_add_action_or_reset(dev, dev_dax_kill, dev_dax);
+	if (rc)
+		return rc;
+
+	/* Probe can no longer fail; expose the pgmap via dev_dax. */
+	dev_dax->pgmap = pgmap;
+	return 0;
 }
 
 static struct dax_device_driver device_dax_driver = {
-- 
2.34.1


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

* Re: [PATCH] dax/device: defer publishing the dynamic pgmap
  2026-09-27 11:47 [PATCH] dax/device: defer publishing the dynamic pgmap Jiale Yao
@ 2026-09-27 11:58 ` sashiko-bot
  2026-09-27 20:05 ` Andrew Morton
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-09-27 11:58 UTC (permalink / raw)
  To: Jiale Yao; +Cc: linux-cxl, nvdimm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dax: NULL pointer dereference in dax_set_mapping() due to probe race
--

commit e1fc62ba8d484aee08a80e703a3d5b11181f82a7
Author: Jiale Yao <yaojiale02@163.com>

dax/device: defer publishing the dynamic pgmap
    
The dynamic device-dax probe path publishes its devm-allocated pgmap
before several operations that can still fail.  If one of them fails,
devres frees the pgmap while dev_dax->pgmap remains non-NULL.  A later
bind then fails the dynamic-dax invariant check and leaves the device
unusable until its region is recreated.
    
Cc: stable@vger.kernel.org

> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> index d0c9b4e03b477..8cd8e05872c35 100644
> --- a/drivers/dax/device.c
> +++ b/drivers/dax/device.c
[ ... ]
> @@ -450,7 +449,13 @@ static int dev_dax_probe(struct dev_dax *dev_dax)
>  		return rc;
>  
>  	run_dax(dax_dev);
> -	return devm_add_action_or_reset(dev, dev_dax_kill, dev_dax);
> +	rc = devm_add_action_or_reset(dev, dev_dax_kill, dev_dax);
> +	if (rc)
> +		return rc;
> +
> +	/* Probe can no longer fail; expose the pgmap via dev_dax. */
> +	dev_dax->pgmap = pgmap;

[Severity: High]
Does this sequence introduce a NULL pointer dereference race condition?

When run_dax() is called, it sets DAXDEV_ALIVE, which enables the device to
begin accepting userspace page faults. Because cdev_add() was already called
earlier in the probe path, a user application can continually call mmap() and
trigger a page fault during device binding.

If a page fault occurs before dev_dax->pgmap is assigned here, could it reach
dax_set_mapping() and dereference a NULL pointer?

drivers/dax/device.c:dax_set_mapping() {
    ...
	/* mapping is only set on the head */
	if (dev_dax->pgmap->vmemmap_shift)
		nr_pages = 1;
    ...
}

> +	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927114736.1113965-1-yaojiale02@163.com?part=1

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

* Re: [PATCH] dax/device: defer publishing the dynamic pgmap
  2026-09-27 11:47 [PATCH] dax/device: defer publishing the dynamic pgmap Jiale Yao
  2026-09-27 11:58 ` sashiko-bot
@ 2026-09-27 20:05 ` Andrew Morton
  2026-09-29 21:45   ` Alison Schofield
  1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2026-09-27 20:05 UTC (permalink / raw)
  To: Jiale Yao
  Cc: Dan Williams, Vishal Verma, Dave Jiang, Alison Schofield,
	Joao Martins, nvdimm, linux-cxl, linux-kernel, stable

On Sun, 27 Sep 2026 19:47:36 +0800 Jiale Yao <yaojiale02@163.com> wrote:

> The dynamic device-dax probe path publishes its devm-allocated pgmap
> before several operations that can still fail.  If one of them fails,
> devres frees the pgmap while dev_dax->pgmap remains non-NULL.  A later
> bind then fails the dynamic-dax invariant check and leaves the device
> unusable until its region is recreated.
> 
> Defer assigning dev_dax->pgmap until the final devm action has been
> installed and probe can no longer fail.  A failed probe then never
> publishes the temporary pgmap.

lgtm, thanks.

> Fixes: fc65c4eb0b2a ("device-dax: ensure dev_dax->pgmap is valid for dynamic devices")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jiale Yao <yaojiale02@163.com>

I'll queue this for next-merge-window as it doesn't seem very urgent. 
Retaining cc:stable.  I've marked it as "needs maintainer acks".


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

* Re: [PATCH] dax/device: defer publishing the dynamic pgmap
  2026-09-27 20:05 ` Andrew Morton
@ 2026-09-29 21:45   ` Alison Schofield
  2026-09-29 21:55     ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: Alison Schofield @ 2026-09-29 21:45 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jiale Yao, Dan Williams, Vishal Verma, Dave Jiang, Joao Martins,
	nvdimm, linux-cxl, linux-kernel, stable

On Sun, Sep 27, 2026 at 01:05:26PM -0700, Andrew Morton wrote:
> On Sun, 27 Sep 2026 19:47:36 +0800 Jiale Yao <yaojiale02@163.com> wrote:
> 
> > The dynamic device-dax probe path publishes its devm-allocated pgmap
> > before several operations that can still fail.  If one of them fails,
> > devres frees the pgmap while dev_dax->pgmap remains non-NULL.  A later
> > bind then fails the dynamic-dax invariant check and leaves the device
> > unusable until its region is recreated.
> > 
> > Defer assigning dev_dax->pgmap until the final devm action has been
> > installed and probe can no longer fail.  A failed probe then never
> > publishes the temporary pgmap.
> 
> lgtm, thanks.
> 
> > Fixes: fc65c4eb0b2a ("device-dax: ensure dev_dax->pgmap is valid for dynamic devices")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jiale Yao <yaojiale02@163.com>
> 
> I'll queue this for next-merge-window as it doesn't seem very urgent. 
> Retaining cc:stable.  I've marked it as "needs maintainer acks".

Hi Andrew,

I'm wondering why you are picking this one up?  I have taken
on the role of drivers/dax patch wrangling recently (past 2 releases)
and was thinking this fits within that group. Not that I don't like
the help, just wondering if I've misunderstood who does what where.

-- Alison


> 

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

* Re: [PATCH] dax/device: defer publishing the dynamic pgmap
  2026-09-29 21:45   ` Alison Schofield
@ 2026-09-29 21:55     ` Andrew Morton
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2026-09-29 21:55 UTC (permalink / raw)
  To: Alison Schofield
  Cc: Jiale Yao, Dan Williams, Vishal Verma, Dave Jiang, Joao Martins,
	nvdimm, linux-cxl, linux-kernel, stable

On Tue, 29 Sep 2026 14:45:15 -0700 Alison Schofield <alison.schofield@intel.com> wrote:

> On Sun, Sep 27, 2026 at 01:05:26PM -0700, Andrew Morton wrote:
> > On Sun, 27 Sep 2026 19:47:36 +0800 Jiale Yao <yaojiale02@163.com> wrote:
> > 
> > > The dynamic device-dax probe path publishes its devm-allocated pgmap
> > > before several operations that can still fail.  If one of them fails,
> > > devres frees the pgmap while dev_dax->pgmap remains non-NULL.  A later
> > > bind then fails the dynamic-dax invariant check and leaves the device
> > > unusable until its region is recreated.
> > > 
> > > Defer assigning dev_dax->pgmap until the final devm action has been
> > > installed and probe can no longer fail.  A failed probe then never
> > > publishes the temporary pgmap.
> > 
> > lgtm, thanks.
> > 
> > > Fixes: fc65c4eb0b2a ("device-dax: ensure dev_dax->pgmap is valid for dynamic devices")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Jiale Yao <yaojiale02@163.com>
> > 
> > I'll queue this for next-merge-window as it doesn't seem very urgent. 
> > Retaining cc:stable.  I've marked it as "needs maintainer acks".
> 
> Hi Andrew,
> 
> I'm wondering why you are picking this one up?  I have taken
> on the role of drivers/dax patch wrangling recently (past 2 releases)
> and was thinking this fits within that group. Not that I don't like
> the help, just wondering if I've misunderstood who does what where.

Oh, OK.  Muscle memory?  I've handled quite a lot of drivers/dax
patches in the past, including the one which is being fixed here.  The
subsystem has never felt more than somewhat on-topic.

I'll remove this one from mm.git.

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

end of thread, other threads:[~2026-09-29 21:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-27 11:47 [PATCH] dax/device: defer publishing the dynamic pgmap Jiale Yao
2026-09-27 11:58 ` sashiko-bot
2026-09-27 20:05 ` Andrew Morton
2026-09-29 21:45   ` Alison Schofield
2026-09-29 21:55     ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox