netdev - Re: [PATCH v9 15/27] cxl: define a driver interface for HPA free space enumeration

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: <67903147bc715_20fa2942e@dwillia2-xfh.jf.intel.com.notmuch>
Date: Tue, 21 Jan 2025 15:44:07 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Alejandro Lucero Palau <alucerop@....com>, Dan Williams
	<dan.j.williams@...el.com>, <alejandro.lucero-palau@....com>,
	<linux-cxl@...r.kernel.org>, <netdev@...r.kernel.org>, <edward.cree@....com>,
	<davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<edumazet@...gle.com>, <dave.jiang@...el.com>
Subject: Re: [PATCH v9 15/27] cxl: define a driver interface for HPA free
 space enumeration

Alejandro Lucero Palau wrote:
[..]
> >> So, I am not sure this code path has ever been tested as lockdep should
> >> complain about the double acquisition.
> >
> >
> > Oddly enough, it has been tested with two different drivers and with 
> > the kernel configuring lockdep.
> >
> > It is worth to investigate ...
> >
> 
> Confirmed the double lock is not an issue. Maybe the code hidden in 
> those macros is checking if the current caller is the same one that the 
> current owner of the lock. I will check that or investigate further.

Are you sure?

This splat:

 ============================================
 WARNING: possible recursive locking detected
 6.13.0-rc2+ #68 Tainted: G           OE     
 --------------------------------------------
 cat/1212 is trying to acquire lock:
 ffffffffc0591cf0 (cxl_region_rwsem){++++}-{4:4}, at: decoders_committed_show+0x2a/0x90 [cxl_core]
 
 but task is already holding lock:
 ffffffffc0591cf0 (cxl_region_rwsem){++++}-{4:4}, at: decoders_committed_show+0x1e/0x90 [cxl_core]
 
 other info that might help us debug this:
  Possible unsafe locking scenario:
 
        CPU0
        ----
   lock(cxl_region_rwsem);
   lock(cxl_region_rwsem);
 
  *** DEADLOCK ***


...results from this change:

diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 72950f631d49..9ebe9d46422b 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -560,9 +560,11 @@ static ssize_t decoders_committed_show(struct device *dev,
        struct cxl_port *port = to_cxl_port(dev);
        int rc;
 
+       down_read(&cxl_region_rwsem);
        down_read(&cxl_region_rwsem);
        rc = sysfs_emit(buf, "%d\n", cxl_num_decoders_committed(port));
        up_read(&cxl_region_rwsem);
+       up_read(&cxl_region_rwsem);
 
        return rc;
 }

...and "cat /sys/bus/cxl/devices/port*/decoders_committed".