* [PATCH] scsi: qedi: fix OOB read in login/text response completion
@ 2026-10-27 15:07 Henry Martin
2026-10-27 15:23 ` sashiko-bot
0 siblings, 1 reply; 3+ messages in thread
From: Henry Martin @ 2026-10-27 15:07 UTC (permalink / raw)
To: Nilesh Javali, Manish Rangankar, GR-QLogic-Storage-Upstream,
James E . J . Bottomley, Martin K . Petersen
Cc: linux-scsi, linux-kernel, Henry Martin, stable
qedi_process_text_resp() and qedi_process_login_resp() take the payload
length straight from the CQE field (hdr_second_dword &
ISCSI_{LOGIN,TEXT}_RESPONSE_HDR_DATA_SEG_LEN_MASK, 24 bits) written
back by the adapter, without any bounds check:
pld_len = cqe_text_response->hdr_second_dword &
ISCSI_TEXT_RESPONSE_HDR_DATA_SEG_LEN_MASK;
qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
...
__iscsi_complete_pdu(conn, ..., qedi_conn->gen_pdu.resp_buf,
(qedi_conn->gen_pdu.resp_wr_ptr -
qedi_conn->gen_pdu.resp_buf));
resp_buf is an 8192-byte buffer (qedi_iscsi.c). A CQE whose data
segment length exceeds the buffer makes resp_wr_ptr point past its end,
and the fabricated length is then passed to __iscsi_complete_pdu() as
the response buffer size: an out-of-bounds read of the buffer (and
beyond) delivered to the iscsi userspace daemon over the iscsi_recv_pdu
netlink unicast.
Clamp the payload length to the response buffer size before computing
the write pointer, in both the login and text response handlers.
This issue was discovered by Tencent CodeBuddy Security.
Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI driver framework.")
Cc: stable@vger.kernel.org
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
drivers/scsi/qedi/qedi_fw.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
index 854efa4f61d87..24724fcba6571 100644
--- a/drivers/scsi/qedi/qedi_fw.c
+++ b/drivers/scsi/qedi/qedi_fw.c
@@ -115,6 +115,8 @@ static void qedi_process_text_resp(struct qedi_ctx *qedi,
pld_len = cqe_text_response->hdr_second_dword &
ISCSI_TEXT_RESPONSE_HDR_DATA_SEG_LEN_MASK;
+ if (pld_len > qedi_conn->gen_pdu.resp_buf_size)
+ pld_len = qedi_conn->gen_pdu.resp_buf_size;
qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
memset(task_ctx, '\0', sizeof(*task_ctx));
@@ -292,6 +294,8 @@ static void qedi_process_login_resp(struct qedi_ctx *qedi,
resp_hdr_ptr->status_detail = cqe_login_response->status_detail;
pld_len = cqe_login_response->hdr_second_dword &
ISCSI_LOGIN_RESPONSE_HDR_DATA_SEG_LEN_MASK;
+ if (pld_len > qedi_conn->gen_pdu.resp_buf_size)
+ pld_len = qedi_conn->gen_pdu.resp_buf_size;
qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
spin_lock(&qedi_conn->list_lock);
--
2.43.7
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] scsi: qedi: fix OOB read in login/text response completion
2026-10-27 15:07 [PATCH] scsi: qedi: fix OOB read in login/text response completion Henry Martin
@ 2026-10-27 15:23 ` sashiko-bot
2026-10-27 15:44 ` henry martin
0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-10-27 15:23 UTC (permalink / raw)
To: Henry Martin; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: qedi: unclamped dlength in qedi_process_text_resp() and login_resp()
--
commit b81ce3a14651452d3f551e4635d2337a8b305729
Author: Henry Martin <bsdhenrymartin@gmail.com>
scsi: qedi: fix OOB read in login/text response completion
This commit clamps the payload length extracted from the CQE in
qedi_process_text_resp() and qedi_process_login_resp() to
qedi_conn->gen_pdu.resp_buf_size before advancing resp_wr_ptr, preventing
an out-of-bounds read when completing the PDU to userspace.
> diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
> index c8af10c326532..5e20716043aee 100644
> --- a/drivers/scsi/qedi/qedi_fw.c
> +++ b/drivers/scsi/qedi/qedi_fw.c
> @@ -115,6 +115,8 @@ static void qedi_process_text_resp(struct qedi_ctx *qedi,
>
> pld_len = cqe_text_response->hdr_second_dword &
> ISCSI_TEXT_RESPONSE_HDR_DATA_SEG_LEN_MASK;
> + if (pld_len > qedi_conn->gen_pdu.resp_buf_size)
> + pld_len = qedi_conn->gen_pdu.resp_buf_size;
> qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
>
> memset(task_ctx, '\0', sizeof(*task_ctx));
> @@ -294,6 +296,8 @@ static void qedi_process_login_resp(struct qedi_ctx *qedi,
> resp_hdr_ptr->status_detail = cqe_login_response->status_detail;
> pld_len = cqe_login_response->hdr_second_dword &
> ISCSI_LOGIN_RESPONSE_HDR_DATA_SEG_LEN_MASK;
> + if (pld_len > qedi_conn->gen_pdu.resp_buf_size)
> + pld_len = qedi_conn->gen_pdu.resp_buf_size;
> qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
[Severity: Medium]
Should resp_hdr_ptr->dlength also be populated from the clamped pld_len in
both qedi_process_text_resp() and qedi_process_login_resp()?
Earlier in both functions, resp_hdr_ptr->dlength is written using the raw
unclamped 24-bit CQE value before pld_len is clamped to
qedi_conn->gen_pdu.resp_buf_size:
drivers/scsi/qedi/qedi_fw.c:qedi_process_text_resp() {
...
hton24(resp_hdr_ptr->dlength,
(cqe_text_response->hdr_second_dword &
ISCSI_TEXT_RESPONSE_HDR_DATA_SEG_LEN_MASK));
...
pld_len = cqe_text_response->hdr_second_dword &
ISCSI_TEXT_RESPONSE_HDR_DATA_SEG_LEN_MASK;
if (pld_len > qedi_conn->gen_pdu.resp_buf_size)
pld_len = qedi_conn->gen_pdu.resp_buf_size;
qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
and similarly in qedi_process_login_resp():
drivers/scsi/qedi/qedi_fw.c:qedi_process_login_resp() {
...
hton24(resp_hdr_ptr->dlength,
(cqe_login_response->hdr_second_dword &
ISCSI_LOGIN_RESPONSE_HDR_DATA_SEG_LEN_MASK));
...
pld_len = cqe_login_response->hdr_second_dword &
ISCSI_LOGIN_RESPONSE_HDR_DATA_SEG_LEN_MASK;
if (pld_len > qedi_conn->gen_pdu.resp_buf_size)
pld_len = qedi_conn->gen_pdu.resp_buf_size;
qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
When __iscsi_complete_pdu() calls iscsi_recv_pdu(), the netlink message is
allocated and populated with only data_size (the clamped pld_len) bytes of
payload following the header:
drivers/scsi/scsi_transport_iscsi.c:iscsi_recv_pdu() {
int len = nlmsg_total_size(sizeof(*ev) + sizeof(struct iscsi_hdr) +
data_size);
...
pdu = (char*)ev + sizeof(*ev);
memcpy(pdu, hdr, sizeof(struct iscsi_hdr));
memcpy(pdu + sizeof(struct iscsi_hdr), data, data_size);
If the CQE data segment length exceeds resp_buf_size, won't this leave
resp_hdr_ptr->dlength advertising the larger unclamped length in the
embedded struct iscsi_hdr while only resp_buf_size bytes of payload are
actually present in the netlink message sent to userspace?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261027150752.951217-1-bsdhenrymartin@gmail.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] scsi: qedi: fix OOB read in login/text response completion
2026-10-27 15:23 ` sashiko-bot
@ 2026-10-27 15:44 ` henry martin
0 siblings, 0 replies; 3+ messages in thread
From: henry martin @ 2026-10-27 15:44 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-scsi
Correct, thanks. Confirmed in-tree: push_callback_call() only
propagates -EFAULT from btf_check_subprog_call(), so the -EINVAL
path reaches check_stack_arg_read() with caller->stack_arg_regs
unvalidated or stale - and it is a heap array sized by
out_stack_arg_cnt, so this is a NULL deref or stale-state copy.
One refinement: set_timer_callback_state() (async timer/workqueue)
does not set in_callback_fn, so that check alone misses the
original frame[-1] case. v2 rejects on both:
if (vstate->curframe == 0 || state->in_callback_fn)
pw-bot: cr
Regards,
Henry
<sashiko-bot@kernel.org> 于2026年10月27日周二 23:23写道：
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] scsi: qedi: unclamped dlength in qedi_process_text_resp() and login_resp()
> --
>
> commit b81ce3a14651452d3f551e4635d2337a8b305729
> Author: Henry Martin <bsdhenrymartin@gmail.com>
>
> scsi: qedi: fix OOB read in login/text response completion
>
> This commit clamps the payload length extracted from the CQE in
> qedi_process_text_resp() and qedi_process_login_resp() to
> qedi_conn->gen_pdu.resp_buf_size before advancing resp_wr_ptr, preventing
> an out-of-bounds read when completing the PDU to userspace.
>
> > diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
> > index c8af10c326532..5e20716043aee 100644
> > --- a/drivers/scsi/qedi/qedi_fw.c
> > +++ b/drivers/scsi/qedi/qedi_fw.c
> > @@ -115,6 +115,8 @@ static void qedi_process_text_resp(struct qedi_ctx *qedi,
> >
> > pld_len = cqe_text_response->hdr_second_dword &
> > ISCSI_TEXT_RESPONSE_HDR_DATA_SEG_LEN_MASK;
> > + if (pld_len > qedi_conn->gen_pdu.resp_buf_size)
> > + pld_len = qedi_conn->gen_pdu.resp_buf_size;
> > qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
> >
> > memset(task_ctx, '\0', sizeof(*task_ctx));
> > @@ -294,6 +296,8 @@ static void qedi_process_login_resp(struct qedi_ctx *qedi,
> > resp_hdr_ptr->status_detail = cqe_login_response->status_detail;
> > pld_len = cqe_login_response->hdr_second_dword &
> > ISCSI_LOGIN_RESPONSE_HDR_DATA_SEG_LEN_MASK;
> > + if (pld_len > qedi_conn->gen_pdu.resp_buf_size)
> > + pld_len = qedi_conn->gen_pdu.resp_buf_size;
> > qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
>
> [Severity: Medium]
> Should resp_hdr_ptr->dlength also be populated from the clamped pld_len in
> both qedi_process_text_resp() and qedi_process_login_resp()?
>
> Earlier in both functions, resp_hdr_ptr->dlength is written using the raw
> unclamped 24-bit CQE value before pld_len is clamped to
> qedi_conn->gen_pdu.resp_buf_size:
>
> drivers/scsi/qedi/qedi_fw.c:qedi_process_text_resp() {
> ...
> hton24(resp_hdr_ptr->dlength,
> (cqe_text_response->hdr_second_dword &
> ISCSI_TEXT_RESPONSE_HDR_DATA_SEG_LEN_MASK));
> ...
> pld_len = cqe_text_response->hdr_second_dword &
> ISCSI_TEXT_RESPONSE_HDR_DATA_SEG_LEN_MASK;
> if (pld_len > qedi_conn->gen_pdu.resp_buf_size)
> pld_len = qedi_conn->gen_pdu.resp_buf_size;
> qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
>
> and similarly in qedi_process_login_resp():
>
> drivers/scsi/qedi/qedi_fw.c:qedi_process_login_resp() {
> ...
> hton24(resp_hdr_ptr->dlength,
> (cqe_login_response->hdr_second_dword &
> ISCSI_LOGIN_RESPONSE_HDR_DATA_SEG_LEN_MASK));
> ...
> pld_len = cqe_login_response->hdr_second_dword &
> ISCSI_LOGIN_RESPONSE_HDR_DATA_SEG_LEN_MASK;
> if (pld_len > qedi_conn->gen_pdu.resp_buf_size)
> pld_len = qedi_conn->gen_pdu.resp_buf_size;
> qedi_conn->gen_pdu.resp_wr_ptr = qedi_conn->gen_pdu.resp_buf + pld_len;
>
> When __iscsi_complete_pdu() calls iscsi_recv_pdu(), the netlink message is
> allocated and populated with only data_size (the clamped pld_len) bytes of
> payload following the header:
>
> drivers/scsi/scsi_transport_iscsi.c:iscsi_recv_pdu() {
> int len = nlmsg_total_size(sizeof(*ev) + sizeof(struct iscsi_hdr) +
> data_size);
> ...
> pdu = (char*)ev + sizeof(*ev);
> memcpy(pdu, hdr, sizeof(struct iscsi_hdr));
> memcpy(pdu + sizeof(struct iscsi_hdr), data, data_size);
>
> If the CQE data segment length exceeds resp_buf_size, won't this leave
> resp_hdr_ptr->dlength advertising the larger unclamped length in the
> embedded struct iscsi_hdr while only resp_buf_size bytes of payload are
> actually present in the netlink message sent to userspace?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261027150752.951217-1-bsdhenrymartin@gmail.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-27 15:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-27 15:07 [PATCH] scsi: qedi: fix OOB read in login/text response completion Henry Martin
2026-10-27 15:23 ` sashiko-bot
2026-10-27 15:44 ` henry martin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox