* [PATCH v3 0/3] scsi: arcmsr: bound adapter-supplied completion indices and message lengths
@ 2026-10-12 11:41 Yehyeong Lee
2026-10-12 11:41 ` [PATCH v3 1/3] scsi: arcmsr: bound the firmware-supplied message length Yehyeong Lee
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Yehyeong Lee @ 2026-10-12 11:41 UTC (permalink / raw)
To: ching2048, mkp, James.Bottomley
Cc: linux-scsi, linux-kernel, stable, thenzl, Yehyeong Lee
The arcmsr completion and message paths take several values straight
from the DMA area the adapter writes -- a completion slot number, a
type B completion-ring index, and an inbound message length -- and use
them as array subscripts or copy lengths without checking them against
the array or buffer they index. A malfunctioning or malicious adapter
can therefore drive an out-of-bounds access from the completion
interrupt or an ioctl.
Patch 1 clamps the firmware-supplied message length to the staging
window (heap out-of-bounds write). Patch 2 rejects a completion slot
the driver never handed out (wild pointer load and dereference on types
E and F). Patch 3 wraps the type B completion index before it is used
(out-of-bounds read, and an out-of-bounds store when the word read is
non-zero).
Each was reproduced on a KASAN build with a crafted DMA area; the
per-patch changelogs have the reports. The three are independent and
carry separate Fixes tags.
v1: https://lore.kernel.org/linux-scsi/cover.1791789900.git.yhlee@isslab.korea.ac.kr/
v2: https://lore.kernel.org/linux-scsi/cover.1791792888.git.yhlee@isslab.korea.ac.kr/
v3: patch 2 -- bound against maxFreeCCB (not the pool size) and clamp
maxFreeCCB to the pool at init, so the index cannot hit an
unpopulated (NULL) slot; v2's ARCMSR_MAX_FREECCB_NUM bound could.
patches 1, 3 unchanged.
v2: patch 2 -- bound against ARCMSR_MAX_FREECCB_NUM rather than
acb->maxFreeCCB, and on a reject release the entry and step the
queue index instead of stopping, so the consumer index does not fall
behind the producer. patches 1, 3 unchanged.
Yehyeong Lee (3):
scsi: arcmsr: bound the firmware-supplied message length
scsi: arcmsr: check the completion index the adapter reports
scsi: arcmsr: wrap the type B completion index before using it
drivers/scsi/arcmsr/arcmsr_hba.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/3] scsi: arcmsr: bound the firmware-supplied message length
2026-10-12 11:41 [PATCH v3 0/3] scsi: arcmsr: bound adapter-supplied completion indices and message lengths Yehyeong Lee
@ 2026-10-12 11:41 ` Yehyeong Lee
2026-10-12 11:54 ` sashiko-bot
2026-10-12 11:41 ` [PATCH v3 2/3] scsi: arcmsr: check the completion index the adapter reports Yehyeong Lee
2026-10-12 11:41 ` [PATCH v3 3/3] scsi: arcmsr: wrap the type B completion index before using it Yehyeong Lee
2 siblings, 1 reply; 8+ messages in thread
From: Yehyeong Lee @ 2026-10-12 11:41 UTC (permalink / raw)
To: ching2048, mkp, James.Bottomley
Cc: linux-scsi, linux-kernel, stable, thenzl, Yehyeong Lee
The adapter hands the driver an inbound message through a fixed
128-byte window:
struct QBUFFER {
uint32_t data_len;
uint8_t data[124];
};
arcmsr_Read_iop_rqbuffer_in_DWORD() takes data_len from that window and
uses it as the copy length without comparing it against the window:
iop_len = readl(&prbuffer->data_len);
if (iop_len > 0) {
buf1 = kmalloc(128, GFP_ATOMIC);
...
data_len = iop_len;
while (data_len >= 4) {
*buf2++ = readl(iop_data);
iop_data++;
data_len -= 4;
}
A data_len above 128 therefore walks the 128-byte allocation, writing
firmware-supplied data past its end, and the loop that follows reads the
same overrun back out of buf1. arcmsr_Read_iop_rqbuffer_data() has the
same unchecked loop for adapter types A and B; there the destination is
wrapped to ARCMSR_MAX_QBUFFER so nothing is written out of bounds, but
the loop still spins up to 2^32 times with rqbuffer_lock held.
Two of the three callers place no bound on the value at all:
arcmsr_iop_message_xfer() for ARCMSR_MESSAGE_READ_RQBUFFER and
arcmsr_sysfs_iop_message_read() both call straight into the reader on
ACB_F_IOPDATA_OVERFLOW. The third, arcmsr_iop2drv_data_wrote_handle(),
compares data_len against the free space in acb->rqbuffer, which is up
to ARCMSR_MAX_QBUFFER - 1 and so still far larger than the 128-byte
staging buffer; it also reads data_len separately from the reader, so
the adapter can change it between the two reads.
Clamp the length to the size of the window in both readers. A
conforming adapter never reports more than sizeof(prbuffer->data), so
this does not drop any data it sends.
With a KASAN build and a crafted window, data_len = 4096 produces 992
out-of-bounds 4-byte writes (1024 dwords less the 32 that fit) in
arcmsr_Read_iop_rqbuffer_in_DWORD():
BUG: KASAN: slab-out-of-bounds in arcmsr_Read_iop_rqbuffer_in_DWORD+0x240/0x2a0 [arcmsr]
Write of size 4 at addr ffff88800b5baf80 by task init/1
Call Trace:
<TASK>
dump_stack_lvl+0x66/0xa0
print_report+0xd0/0x630
kasan_report+0xe4/0x120
arcmsr_Read_iop_rqbuffer_in_DWORD+0x240/0x2a0 [arcmsr]
</TASK>
Allocated by task 1:
__kasan_kmalloc+0x8f/0xa0
__kmalloc_cache_noprof+0x22c/0x570
arcmsr_Read_iop_rqbuffer_in_DWORD+0x7b/0x2a0 [arcmsr]
The buggy address belongs to the object at ffff88800b5baf00
which belongs to the cache kmalloc-128 of size 128
The buggy address is located 0 bytes to the right of
The first value that overruns is data_len = 129; 128 and below are
reported clean, and 124 (the window) and 120 copy byte-for-byte
identically with and without the clamp.
Fixes: bb263c4ecbb1 ("arcmsr: fix ioctl data read/write error for adapter type C")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
drivers/scsi/arcmsr/arcmsr_hba.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 8aa948f06cacf..255f1d898079f 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -2277,6 +2277,8 @@ arcmsr_Read_iop_rqbuffer_in_DWORD(struct AdapterControlBlock *acb,
iop_data = (uint32_t __iomem *)prbuffer->data;
iop_len = readl(&prbuffer->data_len);
+ if (iop_len > sizeof(prbuffer->data))
+ iop_len = sizeof(prbuffer->data);
if (iop_len > 0) {
buf1 = kmalloc(128, GFP_ATOMIC);
buf2 = (uint32_t *)buf1;
@@ -2319,6 +2321,8 @@ arcmsr_Read_iop_rqbuffer_data(struct AdapterControlBlock *acb,
return arcmsr_Read_iop_rqbuffer_in_DWORD(acb, prbuffer);
iop_data = (uint8_t __iomem *)prbuffer->data;
iop_len = readl(&prbuffer->data_len);
+ if (iop_len > sizeof(prbuffer->data))
+ iop_len = sizeof(prbuffer->data);
while (iop_len > 0) {
pQbuffer = &acb->rqbuffer[acb->rqbuf_putIndex];
*pQbuffer = readb(iop_data);
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/3] scsi: arcmsr: check the completion index the adapter reports
2026-10-12 11:41 [PATCH v3 0/3] scsi: arcmsr: bound adapter-supplied completion indices and message lengths Yehyeong Lee
2026-10-12 11:41 ` [PATCH v3 1/3] scsi: arcmsr: bound the firmware-supplied message length Yehyeong Lee
@ 2026-10-12 11:41 ` Yehyeong Lee
2026-10-12 11:41 ` [PATCH v3 3/3] scsi: arcmsr: wrap the type B completion index before using it Yehyeong Lee
2 siblings, 0 replies; 8+ messages in thread
From: Yehyeong Lee @ 2026-10-12 11:41 UTC (permalink / raw)
To: ching2048, mkp, James.Bottomley
Cc: linux-scsi, linux-kernel, stable, thenzl, Yehyeong Lee
On adapter types E and F the adapter reports a completed command by
writing its slot number into the completion queue, and the driver turns
that number straight into an array subscript:
cmdSMID = acb->pCompletionQ[doneq_index].cmdSMID;
ccb = acb->pccb_pool[cmdSMID];
cmdSMID is a uint16_t taken from DMA memory the adapter writes, so it
can be anything up to 65535. Nothing compares the two.
arcmsr_drain_donequeue() cannot stand in for the missing check: it
starts by reading pCCB->acb and pCCB->startdone, so by the time it can
notice anything the bad pointer has already been loaded and
dereferenced. arcmsr_hbaF_postqueue_isr() rejects the single value
0xffff because that is its queue-empty sentinel, which leaves
1024..65534 accepted.
Reject an index the driver never handed out, in the two interrupt
handlers and in the polling path.
Bound against acb->maxFreeCCB rather than ARCMSR_MAX_FREECCB_NUM. The
array has ARCMSR_MAX_FREECCB_NUM entries but arcmsr_alloc_ccb_pool()
only fills [0, maxFreeCCB), and shrinks maxFreeCCB further if the pool
has to stop early at a 4GB boundary, so everything from maxFreeCCB up is
a NULL pointer. Comparing against the array size would let those
indices through to be dereferenced.
Also clamp maxFreeCCB to the array in arcmsr_get_firmware_spec(). It is
derived from the host_can_queue module parameter and gets 64 added to it
when it is below ARCMSR_MAX_FREECCB_NUM, so with can_queue in
[961, 1023] it ends up above the array it indexes -- which the pool loop
itself then walks past. With the clamp in place, maxFreeCCB is both the
populated range and a valid bound.
In the interrupt handlers, release the entry and step the queue index
before continuing, the same way the accepted path does. Stopping at the
bad entry instead would leave the consumer index behind the producer,
so the adapter would re-raise the interrupt on an entry the driver has
already refused and the completions queued behind it would never be
reaped.
Measured with a KASAN build and a crafted completion queue. struct
AdapterControlBlock is 17504 bytes with pccb_pool at offset 496, so a
reported slot of 60000 reads 480496 bytes in, past the end of the
controller block entirely:
BUG: KASAN: use-after-free in arcmsr_hbaE_postqueue_isr+0x242/0x2d0 [arcmsr]
Read of size 8 at addr ffff88800bf354f0 by task init/1
and the pointer it picked up then faults in arcmsr_drain_donequeue:
RIP: 0010:arcmsr_drain_donequeue+0x27/0x360 [arcmsr]
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
A slot inside the array but above maxFreeCCB, such as 200 with the
default maxFreeCCB of 192, loads a NULL entry and faults in the same
place:
Oops: general protection fault, probably for non-canonical address
0xdffffc0000000003: 0000 [#1] SMP KASAN NOPTI
RIP: 0010:arcmsr_drain_donequeue+0x27/0x360 [arcmsr]
Both are refused with the check in place, a queue holding a rejected
entry followed by a good one drains to the producer index and the good
completion is reaped, and valid slot numbers are accepted unchanged.
Fixes: 235090241c55 ("scsi: arcmsr: Add code for ACB_ADAPTER_TYPE_E")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
drivers/scsi/arcmsr/arcmsr_hba.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 255f1d898079f..e635b76daae9a 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -2677,6 +2677,14 @@ static void arcmsr_hbaE_postqueue_isr(struct AdapterControlBlock *acb)
pmu = acb->pmuE;
while ((readl(&pmu->reply_post_producer_index) & 0xFFFF) != doneq_index) {
cmdSMID = acb->pCompletionQ[doneq_index].cmdSMID;
+ if (cmdSMID >= acb->maxFreeCCB) {
+ pr_err_ratelimited("arcmsr%d: bad cmdSMID %u from iop\n",
+ acb->host->host_no, cmdSMID);
+ doneq_index++;
+ if (doneq_index >= acb->completionQ_entry)
+ doneq_index = 0;
+ continue;
+ }
ccb = acb->pccb_pool[cmdSMID];
error = (acb->pCompletionQ[doneq_index].cmdFlag
& ARCMSR_CCBREPLY_FLAG_ERROR_MODE1) ? true : false;
@@ -2706,6 +2714,15 @@ static void arcmsr_hbaF_postqueue_isr(struct AdapterControlBlock *acb)
cmdSMID = acb->pCompletionQ[doneq_index].cmdSMID;
if (cmdSMID == 0xffff)
break;
+ if (cmdSMID >= acb->maxFreeCCB) {
+ pr_err_ratelimited("arcmsr%d: bad cmdSMID %u from iop\n",
+ acb->host->host_no, cmdSMID);
+ acb->pCompletionQ[doneq_index].cmdSMID = 0xffff;
+ doneq_index++;
+ if (doneq_index >= acb->completionQ_entry)
+ doneq_index = 0;
+ continue;
+ }
ccb = acb->pccb_pool[cmdSMID];
error = (acb->pCompletionQ[doneq_index].cmdFlag &
ARCMSR_CCBREPLY_FLAG_ERROR_MODE1) ? true : false;
@@ -3572,6 +3589,8 @@ static bool arcmsr_get_firmware_spec(struct AdapterControlBlock *acb)
acb->maxFreeCCB = acb->host->can_queue;
if (acb->maxFreeCCB < ARCMSR_MAX_FREECCB_NUM)
acb->maxFreeCCB += 64;
+ if (acb->maxFreeCCB > ARCMSR_MAX_FREECCB_NUM)
+ acb->maxFreeCCB = ARCMSR_MAX_FREECCB_NUM;
return rtn;
}
@@ -3880,6 +3899,11 @@ static int arcmsr_hbaE_polling_ccbdone(struct AdapterControlBlock *acb,
doneq_index = 0;
acb->doneq_index = doneq_index;
spin_unlock_irqrestore(&acb->doneq_lock, flags);
+ if (cmdSMID >= acb->maxFreeCCB) {
+ pr_err_ratelimited("arcmsr%d: bad cmdSMID %u from iop\n",
+ acb->host->host_no, cmdSMID);
+ continue;
+ }
pCCB = acb->pccb_pool[cmdSMID];
poll_ccb_done |= (pCCB == poll_ccb) ? 1 : 0;
/* check if command done with no error*/
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 3/3] scsi: arcmsr: wrap the type B completion index before using it
2026-10-12 11:41 [PATCH v3 0/3] scsi: arcmsr: bound adapter-supplied completion indices and message lengths Yehyeong Lee
2026-10-12 11:41 ` [PATCH v3 1/3] scsi: arcmsr: bound the firmware-supplied message length Yehyeong Lee
2026-10-12 11:41 ` [PATCH v3 2/3] scsi: arcmsr: check the completion index the adapter reports Yehyeong Lee
@ 2026-10-12 11:41 ` Yehyeong Lee
2026-10-12 11:59 ` sashiko-bot
2 siblings, 1 reply; 8+ messages in thread
From: Yehyeong Lee @ 2026-10-12 11:41 UTC (permalink / raw)
To: ching2048, mkp, James.Bottomley
Cc: linux-scsi, linux-kernel, stable, thenzl, Yehyeong Lee
struct MessageUnit_B lives in the dma_alloc_coherent() area shared with
the adapter, and holds the completion ring next to the index into it:
uint32_t done_qbuffer[ARCMSR_MAX_HBB_POSTQUEUE];
uint32_t postq_index;
uint32_t doneq_index;
arcmsr_hbaB_postqueue_isr() takes doneq_index from there and subscripts
done_qbuffer[] with it, and only afterwards reduces it:
index = reg->doneq_index;
while ((flag_ccb = reg->done_qbuffer[index]) != 0) {
...
reg->done_qbuffer[index] = 0;
index++;
index %= ARCMSR_MAX_HBB_POSTQUEUE;
reg->doneq_index = index;
}
So the first iteration uses the value as it was found, unmasked. An
index beyond ARCMSR_MAX_HBB_POSTQUEUE (264) reads past done_qbuffer[]
and, should the word it finds be non-zero, stores a zero there as well.
arcmsr_hbaB_polling_ccbdone() has the same ordering.
Reduce the index where it is read instead, which leaves the rest of the
loop unchanged.
Measured with a KASAN build. struct MessageUnit_B is 2176 bytes with
done_qbuffer at offset 1056, so a reported index of 600 reads 3456 bytes
in:
BUG: KASAN: slab-out-of-bounds in arcmsr_hbaB_postqueue_isr+0x1f5/0x270 [arcmsr]
Read of size 4 at addr ffff88800c08cd80 by task init/1
...
which belongs to the cache kmalloc-4k of size 4096
The buggy address is located 1280 bytes to the right of
Fixes: 1c57e86d75cf ("[SCSI] arcmsr: initial driver, version 1.20.00.13")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
---
drivers/scsi/arcmsr/arcmsr_hba.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index e635b76daae9a..d13bd0febb81a 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -2568,7 +2568,7 @@ static void arcmsr_hbaB_postqueue_isr(struct AdapterControlBlock *acb)
bool error;
unsigned long cdb_phy_addr;
- index = reg->doneq_index;
+ index = reg->doneq_index % ARCMSR_MAX_HBB_POSTQUEUE;
while ((flag_ccb = reg->done_qbuffer[index]) != 0) {
cdb_phy_addr = (flag_ccb << 5) & 0xffffffff;
if (acb->cdb_phyadd_hipart)
@@ -3671,7 +3671,7 @@ static int arcmsr_hbaB_polling_ccbdone(struct AdapterControlBlock *acb,
/* clear doorbell interrupt */
writel(ARCMSR_DOORBELL_INT_CLEAR_PATTERN, reg->iop2drv_doorbell);
while(1){
- index = reg->doneq_index;
+ index = reg->doneq_index % ARCMSR_MAX_HBB_POSTQUEUE;
flag_ccb = reg->done_qbuffer[index];
if (flag_ccb == 0) {
if (poll_ccb_done){
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/3] scsi: arcmsr: bound the firmware-supplied message length
2026-10-12 11:41 ` [PATCH v3 1/3] scsi: arcmsr: bound the firmware-supplied message length Yehyeong Lee
@ 2026-10-12 11:54 ` sashiko-bot
2026-10-12 12:16 ` Yehyeong Lee
0 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-10-12 11:54 UTC (permalink / raw)
To: Yehyeong Lee; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: arcmsr: TOCTOU double fetch causes circular buffer corruption
--
commit c1b87442346f4c23552402e5fce0db15b1252056
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
scsi: arcmsr: bound the firmware-supplied message length
This commit addresses an out-of-bounds write where an unbounded data_len
from the MMIO register could walk past the 128-byte staging buffer. It
clamps the length to the size of the window (124 bytes) in both reader
functions.
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
> index 8aa948f06cacf..255f1d898079f 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> @@ -2277,6 +2277,8 @@ arcmsr_Read_iop_rqbuffer_in_DWORD(struct AdapterControlBlock *acb,
>
> iop_data = (uint32_t __iomem *)prbuffer->data;
> iop_len = readl(&prbuffer->data_len);
> + if (iop_len > sizeof(prbuffer->data))
> + iop_len = sizeof(prbuffer->data);
[Severity: Medium]
The commit message notes that the adapter can change data_len between the two
reads. While clamping iop_len to sizeof(prbuffer->data) prevents the staging
buffer overflow, does this still leave a TOCTOU (time-of-check to time-of-use)
vulnerability that can corrupt the circular buffer?
arcmsr_iop2drv_data_wrote_handle() validates the first fetch against
buf_empty_len:
drivers/scsi/arcmsr/arcmsr_hba.c:arcmsr_iop2drv_data_wrote_handle() {
...
if (buf_empty_len >= readl(&prbuffer->data_len)) {
if (arcmsr_Read_iop_rqbuffer_data(acb, prbuffer) == 0)
...
}
If buf_empty_len is small (e.g., 10 bytes), the first fetch could be 10, passing
the check. If the second fetch here sees an increased data_len from the MMIO
register (e.g., 124 bytes), the while loop below will write more bytes than
buf_empty_len.
Could this cause rqbuf_putIndex to overtake rqbuf_getIndex and corrupt the
driver's internal ring buffer state?
> if (iop_len > 0) {
> buf1 = kmalloc(128, GFP_ATOMIC);
> buf2 = (uint32_t *)buf1;
[ ... ]
> @@ -2319,6 +2321,8 @@ arcmsr_Read_iop_rqbuffer_data(struct AdapterControlBlock *acb,
> return arcmsr_Read_iop_rqbuffer_in_DWORD(acb, prbuffer);
> iop_data = (uint8_t __iomem *)prbuffer->data;
> iop_len = readl(&prbuffer->data_len);
> + if (iop_len > sizeof(prbuffer->data))
> + iop_len = sizeof(prbuffer->data);
[Severity: Medium]
This second fetch in the byte-wise reader shares the same vulnerability when
called from arcmsr_iop2drv_data_wrote_handle().
Would passing the validated length down from the caller prevent the hardware
from bypassing the buf_empty_len check entirely?
> while (iop_len > 0) {
> pQbuffer = &acb->rqbuffer[acb->rqbuf_putIndex];
> *pQbuffer = readb(iop_data);
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791803953.git.yhlee@isslab.korea.ac.kr?part=1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] scsi: arcmsr: wrap the type B completion index before using it
2026-10-12 11:41 ` [PATCH v3 3/3] scsi: arcmsr: wrap the type B completion index before using it Yehyeong Lee
@ 2026-10-12 11:59 ` sashiko-bot
2026-10-12 12:16 ` Yehyeong Lee
0 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-10-12 11:59 UTC (permalink / raw)
To: Yehyeong Lee; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: arcmsr: infinite loop in ISR processing due to unbounded hardware DMA
--
commit 728fbf206bd1c95a2cfbe31aec39150fac1e1356
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
scsi: arcmsr: wrap the type B completion index before using it
This commit fixes a KASAN slab-out-of-bounds issue in arcmsr_hbaB_postqueue_isr
and arcmsr_hbaB_polling_ccbdone by wrapping the doneq_index with
ARCMSR_MAX_HBB_POSTQUEUE before it is used to index into done_qbuffer[].
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
> index e635b76daae9a..d13bd0febb81a 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> @@ -2568,7 +2568,7 @@ static void arcmsr_hbaB_postqueue_isr(struct AdapterControlBlock *acb)
> bool error;
> unsigned long cdb_phy_addr;
>
> - index = reg->doneq_index;
> + index = reg->doneq_index % ARCMSR_MAX_HBB_POSTQUEUE;
> while ((flag_ccb = reg->done_qbuffer[index]) != 0) {
[Severity: Medium]
Could this loop spin infinitely? The loop condition depends entirely on
hardware-controlled DMA memory without an iteration limit.
If a faulty or malicious PCIe device continuously populates the DMA-mapped
done_qbuffer faster than the CPU processes it, or supplies an endless stream
of non-zero completions, can this lead to a CPU soft or hard lockup?
> cdb_phy_addr = (flag_ccb << 5) & 0xffffffff;
> if (acb->cdb_phyadd_hipart)
[ ... ]
> @@ -3671,7 +3671,7 @@ static int arcmsr_hbaB_polling_ccbdone(struct AdapterControlBlock *acb,
> /* clear doorbell interrupt */
> writel(ARCMSR_DOORBELL_INT_CLEAR_PATTERN, reg->iop2drv_doorbell);
> while(1){
> - index = reg->doneq_index;
> + index = reg->doneq_index % ARCMSR_MAX_HBB_POSTQUEUE;
> flag_ccb = reg->done_qbuffer[index];
> if (flag_ccb == 0) {
[Severity: Medium]
Could this polling loop also lack a structural bound and spin infinitely?
If the hardware continuously supplies non-zero completions, poll_count is only
evaluated when flag_ccb == 0. Can this result in a denial of service if the
hardware never returns a zero completion?
> if (poll_ccb_done){
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791803953.git.yhlee@isslab.korea.ac.kr?part=3
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/3] scsi: arcmsr: bound the firmware-supplied message length
2026-10-12 11:54 ` sashiko-bot
@ 2026-10-12 12:16 ` Yehyeong Lee
0 siblings, 0 replies; 8+ messages in thread
From: Yehyeong Lee @ 2026-10-12 12:16 UTC (permalink / raw)
To: linux-scsi; +Cc: ching2048, mkp, James.Bottomley, thenzl, sashiko-reviews
Agreed, and as you note this predates the patch. The clamp here only stops
the out-of-bounds write into the 128-byte staging buffer and deliberately
leaves the rqbuffer accounting alone.
The double fetch is a separate issue: the free-space check in
arcmsr_iop2drv_data_wrote_handle() and the reader each read data_len on
their own, so the firmware can grow it in between and advance rqbuf_putIndex
past the unread data. rqbuf_putIndex is masked with ARCMSR_MAX_QBUFFER, so
this corrupts the message stream to userspace rather than stepping outside
rqbuffer. I would rather fix that on its own by reading the length once and
passing it down, so this series stays a pure bounds fix; I can send that
separately.
Thanks,
Yehyeong
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] scsi: arcmsr: wrap the type B completion index before using it
2026-10-12 11:59 ` sashiko-bot
@ 2026-10-12 12:16 ` Yehyeong Lee
0 siblings, 0 replies; 8+ messages in thread
From: Yehyeong Lee @ 2026-10-12 12:16 UTC (permalink / raw)
To: linux-scsi; +Cc: ching2048, mkp, James.Bottomley, thenzl, sashiko-reviews
This is pre-existing: the patch only wraps the index so the first access
stays in bounds, and leaves the loop and its termination unchanged.
Both type B loops clear each slot they process (done_qbuffer[index] = 0) and
advance, so the loop only keeps going if the adapter keeps writing fresh
non-zero entries into the ring - a faulty or malicious device forging an
endless stream of completions, which is a weaker and different threat than
the out-of-bounds read this patch closes. The same drain-until-empty
structure is used by every adapter type, and only the type C isr carries an
iteration budget today, so adding one is better done as its own change across
the board rather than folded into this one-line fix.
Thanks,
Yehyeong
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-12 12:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 11:41 [PATCH v3 0/3] scsi: arcmsr: bound adapter-supplied completion indices and message lengths Yehyeong Lee
2026-10-12 11:41 ` [PATCH v3 1/3] scsi: arcmsr: bound the firmware-supplied message length Yehyeong Lee
2026-10-12 11:54 ` sashiko-bot
2026-10-12 12:16 ` Yehyeong Lee
2026-10-12 11:41 ` [PATCH v3 2/3] scsi: arcmsr: check the completion index the adapter reports Yehyeong Lee
2026-10-12 11:41 ` [PATCH v3 3/3] scsi: arcmsr: wrap the type B completion index before using it Yehyeong Lee
2026-10-12 11:59 ` sashiko-bot
2026-10-12 12:16 ` Yehyeong Lee
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox