linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ivy Lopez <skunkolee@gmail.com>
To: aacraid@microsemi.com, James.Bottomley@HansenPartnership.com,
	martin.petersen@oracle.com
Cc: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
	Ivy Lopez <skunkolee@gmail.com>
Subject: [PATCH v2] scsi: aacraid: fix DMA mapping leak in aac_send_raw_srb()
Date: Mon, 10 Aug 2026 16:27:38 -0600	[thread overview]
Message-ID: <20260810222738.26278-1-skunkolee@gmail.com> (raw)
In-Reply-To: <20260809033449.286233-1-skunkolee@gmail.com>

aac_send_raw_srb() maps each scatter/gather entry for DMA via
dma_map_single() across five separate code paths, depending on the
adapter type and SG format (native HBA, 64-bit host SG, 32-bit host
SG, and two legacy formats). None of these mappings are ever undone:
there is no dma_unmap_single() call anywhere in the file, on the
success path or any of the error paths that funnel through the
single cleanup label.

Every FSACTL_SEND_RAW_SRB ioctl that submits at least one SG entry
therefore leaks that many DMA mappings permanently. Under an IOMMU
or SWIOTLB this is a genuinely exhaustible resource: sustained use
(e.g. periodic smartctl -d aacraid,... polling) eventually drives
new DMA mappings to fail, surfacing as intermittent I/O failures
(aac_fib_send failing with -ENOMEM) and, left long enough, adapter
resets and system instability.

Fix this by tracking the DMA address returned from each of the five
dma_map_single() calls in a new per-entry array (sg_addr[]). Each
entry is initialized to DMA_MAPPING_ERROR and checked with
dma_mapping_error() immediately after mapping, bailing out to
cleanup on failure rather than using a possibly-error address.
Entries are unmapped exactly once: for SRB_DataIn (hardware writes
via DMA), each entry is unmapped immediately before its
copy_to_user() in the existing per-entry loop, since on
non-coherent architectures dma_unmap_single() performs the cache
invalidation needed before the CPU can safely read what the device
wrote; unmapping only later in cleanup, after that read, could
return stale pre-DMA-completion data to userspace. Entries handled
this way are marked DMA_MAPPING_ERROR again so cleanup does not
unmap them a second time. All other entries (SRB_DataOut-only, or
any entry on an error path that never reaches the DataIn copy loop)
are unmapped once in cleanup, guarded by the same sentinel check.

v1 of this patch used 0 as the "unmapped" sentinel and unmapped
every entry only in cleanup, after any copy_to_user() had already
read from it. Both were wrong: 0 is a valid DMA address on some
platforms (DMA_MAPPING_ERROR is ~(dma_addr_t)0, not 0), so a mapping
that legitimately returned address 0 would never be unmapped; and
unmapping only in cleanup meant SRB_DataIn transfers could read
stale, not-yet-cache-invalidated data on non-coherent architectures.
Both issues were caught in review by an automated reviewer
(Sashiko AI) on the v1 submission.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=220504
Signed-off-by: Ivy Lopez <skunkolee@gmail.com>
---
 drivers/scsi/aacraid/commctrl.c | 43 +++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index bd82aeb679ae..27482d98c6fd 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -492,6 +492,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 	u32 data_dir;
 	void __user *sg_user[HBA_MAX_SG_EMBEDDED];
 	void *sg_list[HBA_MAX_SG_EMBEDDED];
+	dma_addr_t sg_addr[HBA_MAX_SG_EMBEDDED];
 	u32 sg_count[HBA_MAX_SG_EMBEDDED];
 	u32 sg_indx = 0;
 	u32 byte_count = 0;
@@ -517,6 +518,8 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 	}
 
 	memset(sg_list, 0, sizeof(sg_list)); /* cleanup may take issue */
+	for (i = 0; i < HBA_MAX_SG_EMBEDDED; i++)
+		sg_addr[i] = DMA_MAPPING_ERROR; /* mark all entries unmapped */
 	if(copy_from_user(&fibsize, &user_srb->count,sizeof(u32))){
 		dprintk((KERN_DEBUG"aacraid: Could not copy data size from user\n"));
 		rcode = -EFAULT;
@@ -690,6 +693,11 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 			}
 			addr = dma_map_single(&dev->pdev->dev, p, sg_count[i],
 					      data_dir);
+			if (dma_mapping_error(&dev->pdev->dev, addr)) {
+				rcode = -ENOMEM;
+				goto cleanup;
+			}
+			sg_addr[i] = addr;
 			hbacmd->sge[i].addr_hi = cpu_to_le32((u32)(addr>>32));
 			hbacmd->sge[i].addr_lo = cpu_to_le32(
 						(u32)(addr & 0xffffffff));
@@ -752,7 +760,11 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 				}
 				addr = dma_map_single(&dev->pdev->dev, p,
 						      sg_count[i], data_dir);
-
+				if (dma_mapping_error(&dev->pdev->dev, addr)) {
+					rcode = -ENOMEM;
+					goto cleanup;
+				}
+				sg_addr[i] = addr;
 				psg->sg[i].addr[0] = cpu_to_le32(addr & 0xffffffff);
 				psg->sg[i].addr[1] = cpu_to_le32(addr>>32);
 				byte_count += sg_count[i];
@@ -808,7 +820,11 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 				}
 				addr = dma_map_single(&dev->pdev->dev, p,
 						      sg_count[i], data_dir);
-
+				if (dma_mapping_error(&dev->pdev->dev, addr)) {
+					rcode = -ENOMEM;
+					goto cleanup;
+				}
+				sg_addr[i] = addr;
 				psg->sg[i].addr[0] = cpu_to_le32(addr & 0xffffffff);
 				psg->sg[i].addr[1] = cpu_to_le32(addr>>32);
 				byte_count += sg_count[i];
@@ -865,7 +881,11 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 				addr = dma_map_single(&dev->pdev->dev, p,
 						      usg->sg[i].count,
 						      data_dir);
-
+				if (dma_mapping_error(&dev->pdev->dev, addr)) {
+					rcode = -ENOMEM;
+					goto cleanup;
+				}
+				sg_addr[i] = addr;
 				psg->sg[i].addr = cpu_to_le32(addr & 0xffffffff);
 				byte_count += usg->sg[i].count;
 				psg->sg[i].count = cpu_to_le32(sg_count[i]);
@@ -905,7 +925,11 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 				}
 				addr = dma_map_single(&dev->pdev->dev, p,
 						      sg_count[i], data_dir);
-
+				if (dma_mapping_error(&dev->pdev->dev, addr)) {
+					rcode = -ENOMEM;
+					goto cleanup;
+				}
+				sg_addr[i] = addr;
 				psg->sg[i].addr = cpu_to_le32(addr);
 				byte_count += sg_count[i];
 				psg->sg[i].count = cpu_to_le32(sg_count[i]);
@@ -932,6 +956,11 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 
 	if (flags & SRB_DataIn) {
 		for(i = 0 ; i <= sg_indx; i++){
+			if (sg_addr[i] != DMA_MAPPING_ERROR) {
+				dma_unmap_single(&dev->pdev->dev, sg_addr[i],
+						 sg_count[i], data_dir);
+				sg_addr[i] = DMA_MAPPING_ERROR;
+			}
 			if (copy_to_user(sg_user[i], sg_list[i], sg_count[i])) {
 				dprintk((KERN_DEBUG"aacraid: Could not copy sg data to user\n"));
 				rcode = -EFAULT;
@@ -986,8 +1015,12 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 cleanup:
 	kfree(user_srbcmd);
 	if (rcode != -ERESTARTSYS) {
-		for (i = 0; i <= sg_indx; i++)
+		for (i = 0; i <= sg_indx; i++) {
+			if (sg_addr[i] != DMA_MAPPING_ERROR)
+				dma_unmap_single(&dev->pdev->dev, sg_addr[i],
+						 sg_count[i], data_dir);
 			kfree(sg_list[i]);
+		}
 		aac_fib_complete(srbfib);
 		aac_fib_free(srbfib);
 	}
-- 
2.55.0


      reply	other threads:[~2026-08-10 22:28 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-09  3:34 [PATCH] scsi: aacraid: fix DMA mapping leak in aac_send_raw_srb() Ivy Lopez
2026-08-10 22:27 ` Ivy Lopez [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260810222738.26278-1-skunkolee@gmail.com \
    --to=skunkolee@gmail.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=aacraid@microsemi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).