[PATCH v2] scsi: aacraid: fix DMA mapping leak in aac_send_raw_srb()
Ivy Lopez <[email protected]> Mon, 10 Aug 2026 16:27:38 -0600
| Newsgroups | gmane.linux.scsi,gmane.linux.kernel |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> --- 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