linux-kernel - Re: [PATCH 2/2] cxl/core: Hold grandparent port lock while dport adding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <19c23e33320.c78951ef7300.3027465952765956716@zohomail.com>
Date: Tue, 03 Feb 2026 22:23:38 +0800
From: Li Ming <ming.li@...omail.com>
To: "Jonathan Cameron" <jonathan.cameron@...wei.com>
Cc: "dave" <dave@...olabs.net>, "dave.jiang" <dave.jiang@...el.com>,
	"alison.schofield" <alison.schofield@...el.com>,
	"vishal.l.verma" <vishal.l.verma@...el.com>,
	"ira.weiny" <ira.weiny@...el.com>,
	"dan.j.williams" <dan.j.williams@...el.com>,
	"linux-cxl" <linux-cxl@...r.kernel.org>,
	"linux-kernel" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] cxl/core: Hold grandparent port lock while dport
 adding




From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: "Li Ming"<ming.li@...omail.com>
Cc: <dave@...olabs.net>, <dave.jiang@...el.com>, <alison.schofield@...el.com>, <vishal.l.verma@...el.com>, <ira.weiny@...el.com>, <dan.j.williams@...el.com>, <linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Date: Mon, 02 Feb 2026 23:39:24 +0800
Subject: Re: [PATCH 2/2] cxl/core: Hold grandparent port lock while dport adding

 > On Sun,  1 Feb 2026 17:30:02 +0800
 > Li Ming <ming.li@...omail.com> wrote:
 > 
 > > When CXL subsystem adds a cxl port to a hierarchy, there is a small
 > > window where the new port becomes visible before it is bound to a
 > > driver. This happens because device_add() adds a device to bus device
 > > list before bus_probe_device() binds it to a driver.
 > > So if two cxl memdevs are trying to add a dport to a same port via
 > > devm_cxl_enumerate_ports(), the second cxl memdev may observe the port
 > > and attempt to add a dport, but fails because the port has not yet been
 > > attached to cxl port driver.
 > > the sequence is like:
 > > 
 > > CPU 0                    CPU 1
 > > devm_cxl_enumerate_ports()
 > >   # port not found, add it
 > >   add_port_attach_ep()
 > >     # hold the parent port lock
 > >     # to add the new port
 > >     devm_cxl_create_port()
 > >       device_add()
 > >     # Add dev to bus devs list
 > >     bus_add_device()
 > >                     devm_cxl_enumerate_ports()
 > >                     # found the port
 > 
 > Indenting not consistent here as this call is in devm_cxl_enumerate_ports()
Thanks for that, will fix it in next version.

 > 
 > >                     find_cxl_port_by_uport()
 > >                     # hold port lock to add a dport
 > >                     device_lock(the port)
 > >                     find_or_add_dport()
 > >                       cxl_port_add_dport()
 > >                         return -ENXIO because port->dev.driver is NULL
 > >                     device_unlock(the port)
 > >     bus_probe_device()
 > >       # hold the port lock
 > >       # for attaching
 > >       device_lock(the port)
 > >         attaching the new port
 > >       device_unlock(the port)
 > > 
 > > To fix this race, require that dport addition holds the parent port lock
 > > of the target port. The CXL subsystem already requires holding the
 > > parent port lock while attaching a new port. Therefore, successfully
 > > acquiring the parent port lock ganrantees that port attaching has
 > 
 > Spell check. Guarantees
Got it.

 > 
 > > completed.
 > > 
 > > Fixes: 4f06d81e7c6a ("cxl: Defer dport allocation for switch ports")
 > > Signed-off-by: Li Ming <ming.li@...omail.com>
 > 
 > Analysis looks reasonable to me, but I'm not hugely confident on this
 > one so would like others to take a close look as well.
 > Question inline.
 > 
 > 
 > > ---
 > >  drivers/cxl/core/port.c | 6 +++++-
 > >  1 file changed, 5 insertions(+), 1 deletion(-)
 > > 
 > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
 > > index 54f72452fb06..fef2fe913e1f 100644
 > > --- a/drivers/cxl/core/port.c
 > > +++ b/drivers/cxl/core/port.c
 > > @@ -1817,8 +1817,12 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
 > >              /*
 > >               * RP port enumerated by cxl_acpi without dport will
 > >               * have the dport added here.
 > > +             *
 > > +             * Hold the parent port lock here to in case that the
 > > +             * port can be observed but has not been attached yet.
 > >               */
 > > -            scoped_guard(device, &port->dev) {
 > > +            scoped_guard(device, &parent_port_of(port)->dev) {
 > I'm nervous about whether this is the right lock. For unregister_port()
 > (which is easier to track down that the add path locking) the lock
 > taken depends on where the port is that is being unregistered.
 > Specifically root ports are unregistered under parent->uport_dev, not
 > parent->dev.
You are right.
When cxl acpi driver attempts to add a cxl host bridge port, it will hold cxl_root->uport_dev.
Otherwide, hold parent_port->dev lock for the new port addition.
So I think it is possible that memdev can observe a cxl host bridge port but it has not been attach yet.
Maybe I should hold the lock of the new port's host.
Let's see if other reviewers have more comments on that.

Ming
 > 
 > > +                guard(device)(&port->dev);
 > >                  dport = find_or_add_dport(port, dport_dev);
 > >                  if (IS_ERR(dport)) {
 > >                      if (PTR_ERR(dport) == -EAGAIN)
 > 
 >