linux-kernel - RE: [PATCH v8 1/1] dmaengine: switchtec-dma: Introduce Switchtec DMA engine PCI driver

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]

Date: Thu, 21 Mar 2024 16:21:19 +0000
From: "Grochowski, Maciej" <Maciej.Grochowski@...y.com>
To: Christoph Hellwig <hch@...radead.org>,
        Kelvin Cao
	<kelvin.cao@...rochip.com>
CC: "vkoul@...nel.org" <vkoul@...nel.org>,
        "dmaengine@...r.kernel.org"
	<dmaengine@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "logang@...tatee.com" <logang@...tatee.com>,
        "George.Ge@...rochip.com" <George.Ge@...rochip.com>,
        "christophe.jaillet@...adoo.fr" <christophe.jaillet@...adoo.fr>
Subject: RE: [PATCH v8 1/1] dmaengine: switchtec-dma: Introduce Switchtec DMA
 engine PCI driver

Hi Kelvin,

I have noticed lockdep_rcu complaining about suspicious rcu_dereference inside: switchtec_dma_chans_enumerate and switchtec_dma_chan_free functions

In the switchtec_dma_chans_enumerate:
```
static int switchtec_dma_chans_enumerate(struct switchtec_dma_dev *swdma_dev,
                                          int chan_cnt)
 {
         struct dma_device *dma = &swdma_dev->dma_dev;
         struct pci_dev *pdev = rcu_dereference(swdma_dev->pdev);	<<< suspicious RCU usage
         int base;
         int cnt;
         int rc;
         int i;
```
And inside switchtec_dma_chan_free
```
static int switchtec_dma_chan_free(struct switchtec_dma_chan
 *swdma_chan) {
         struct pci_dev *pdev = rcu_dereference(swdma_chan->swdma_dev->pdev);  <<< suspicious RCU usage

         spin_lock_bh(&swdma_chan->submit_lock);
         swdma_chan->ring_active = false;
         spin_unlock_bh(&swdma_chan->submit_lock);
```

Is going to trigger lockdep_rcu below log from my system:
```
[  296.678725] =============================
[  296.682890] WARNING: suspicious RCU usage
[  296.687017] 6.8.0-rc4+ #2 Tainted: G            E
[  296.687028] -----------------------------
[  296.691166] drivers/dma/switchtec_dma.c:1256 suspicious rcu_dereference_check() usage!
[  296.699263]
               other info that might help us debug this:

[  296.707446]
               rcu_scheduler_active = 2, debug_locks = 1
[  296.714142] 1 lock held by insmod/1536:
[  296.714153]  #0: ffff8881191d91b0 (&dev->mutex){....}-{4:4}, at: __driver_attach+0x172/0x3b0
[  296.714207]
               stack backtrace:
[  296.718689] CPU: 45 PID: 1536 Comm: insmod Tainted: G            E      6.8.0-rc4+ #2
[  296.718710] Call Trace:
[  296.718720]  <TASK>
[  296.718731]  dump_stack_lvl+0xac/0xc0
[  296.718750]  lockdep_rcu_suspicious+0x1dd/0x380
[  296.718782]  switchtec_dma_probe+0xf0a/0x1400 [switchtec_dma]
[  296.718819]  ? _raw_spin_unlock_irqrestore+0x66/0x80
```

I think it should be also guarded by rcu_read_lock/unlock in order to avoid such situations.