* [PATCH v2 1/5] scsi: ufs: bsg: Complete the job on error
2026-10-15 0:18 [PATCH v2 0/5] scsi: ufs: Fix UIC result byte and BSG request handling Stanley Jhu
@ 2026-10-15 0:18 ` Stanley Jhu
2026-10-15 0:32 ` sashiko-bot
2026-10-15 0:18 ` [PATCH v2 2/5] scsi: ufs: bsg: Clear the reply buffer before use Stanley Jhu
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Stanley Jhu @ 2026-10-15 0:18 UTC (permalink / raw)
To: Martin K. Petersen, James E.J. Bottomley
Cc: Alim Akhtar, Bart Van Assche, Bean Huo, Peter Wang, Brian Kao,
linux-scsi, linux-kernel, stable
When ufs_bsg_request() fails, it returns a non-zero status without
calling bsg_job_done(). bsg_queue_rq() then ends the request with
BLK_STS_IOERR without running bsg_complete(), leaking the job's device
reference and mapped scatterlists.
Always complete the job via bsg_job_done() and return 0, matching other
BSG transports. Requests failing with a negative errno now return it
from SG_IO with the 4-byte result in the reply, while positive status
codes still return 0 from SG_IO with the reply header.
Fixes: b13a3539eb2a ("scsi: ufs-bsg: complete ufs-bsg job only if no error")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
drivers/ufs/core/ufs_bsg.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 7a66917b3fc0..18c318d78122 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -195,14 +195,12 @@ static int ufs_bsg_request(struct bsg_job *job)
ufshcd_rpm_put_sync(hba);
kfree(buff);
bsg_reply->result = ret;
- /* complete the job here only if no error */
- if (ret == 0) {
+ if (ret == 0)
job->reply_len = rpmb ? sizeof(struct ufs_rpmb_reply) :
sizeof(struct ufs_bsg_reply);
- bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
- }
+ bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
- return ret;
+ return 0;
}
/**
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/5] scsi: ufs: bsg: Complete the job on error
2026-10-15 0:18 ` [PATCH v2 1/5] scsi: ufs: bsg: Complete the job on error Stanley Jhu
@ 2026-10-15 0:32 ` sashiko-bot
2026-10-15 0:41 ` Stanley Jhu
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-10-15 0:32 UTC (permalink / raw)
To: Stanley Jhu; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ufs: bsg: heap out-of-bounds write/read in RPMB reply
--
commit 500adcab38ae23977180188472414e7c66ab1588
Author: Stanley Jhu <stanleyjhu@google.com>
scsi: ufs: bsg: Complete the job on error
This patch updates ufs_bsg_request to always complete the job via
bsg_job_done() and return 0, matching other BSG transports. It prevents
leaking the job's device reference and mapped scatterlists on error.
Positive status codes continue to return 0 from SG_IO with the reply header.
Cc: stable@vger.kernel.org
> diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
> index 7a66917b3fc0e..18c318d781224 100644
> --- a/drivers/ufs/core/ufs_bsg.c
> +++ b/drivers/ufs/core/ufs_bsg.c
> @@ -195,14 +195,12 @@ static int ufs_bsg_request(struct bsg_job *job)
> ufshcd_rpm_put_sync(hba);
> kfree(buff);
> bsg_reply->result = ret;
> - /* complete the job here only if no error */
> - if (ret == 0) {
> + if (ret == 0)
> job->reply_len = rpmb ? sizeof(struct ufs_rpmb_reply) :
> sizeof(struct ufs_bsg_reply);
[Severity: High]
Does setting job->reply_len to sizeof(struct ufs_rpmb_reply) (which is 104
bytes) cause an out-of-bounds write and read?
The job->reply buffer is allocated with a fixed size of 96 bytes
(SCSI_SENSE_BUFFERSIZE) in bsg_init_rq().
During a UFS Advanced RPMB request, ufshcd_advanced_rpmb_req_handler() casts
the 96-byte job->reply buffer to a struct ufs_rpmb_reply and writes up to
104 bytes, overflowing the allocation by 8 bytes.
Later, bsg_complete_op() uses job->reply_len to copy the reply back to
userspace:
block/bsg-lib.c:bsg_complete_op() {
...
int len = min(hdr->max_response_len, job->reply_len);
if (copy_to_user(uptr64(hdr->response), job->reply, len))
...
}
Because job->reply_len is set to 104, this copies 8 bytes of adjacent
slab memory to userspace, which could lead to heap corruption and
memory disclosure.
> - bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
> - }
> + bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
>
> - return ret;
> + return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261015001855.3450573-1-stanleyjhu@google.com?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/5] scsi: ufs: bsg: Complete the job on error
2026-10-15 0:32 ` sashiko-bot
@ 2026-10-15 0:41 ` Stanley Jhu
0 siblings, 0 replies; 13+ messages in thread
From: Stanley Jhu @ 2026-10-15 0:41 UTC (permalink / raw)
To: Martin K. Petersen, James E.J. Bottomley
Cc: Alim Akhtar, Bart Van Assche, Bean Huo, Peter Wang, Brian Kao,
linux-scsi, linux-kernel
On Thu, Oct 15, 2026 at 12:32:09AM +0000, sashiko-bot@kernel.org wrote:
> Because job->reply_len is set to 104, this copies 8 bytes of adjacent
> slab memory to userspace, which could lead to heap corruption and
> memory disclosure.
Pre-existing and not changed by this patch; fixed separately in bsg-lib:
https://lore.kernel.org/all/20261013132708.1669415-1-stanleyjhu@google.com/
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/5] scsi: ufs: bsg: Clear the reply buffer before use
2026-10-15 0:18 [PATCH v2 0/5] scsi: ufs: Fix UIC result byte and BSG request handling Stanley Jhu
2026-10-15 0:18 ` [PATCH v2 1/5] scsi: ufs: bsg: Complete the job on error Stanley Jhu
@ 2026-10-15 0:18 ` Stanley Jhu
2026-10-15 0:34 ` sashiko-bot
2026-10-15 0:18 ` [PATCH v2 3/5] scsi: ufs: bsg: Validate request_len before accessing request Stanley Jhu
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Stanley Jhu @ 2026-10-15 0:18 UTC (permalink / raw)
To: Martin K. Petersen, James E.J. Bottomley
Cc: Alim Akhtar, Bart Van Assche, Bean Huo, Peter Wang, Brian Kao,
linux-scsi, linux-kernel, stable
The reply buffer is allocated once per tag and reused. Requests that
complete with a positive status do not fill all of it, so user space
receives bytes from an earlier request on the same tag.
Clear the whole reply buffer on entry to ufs_bsg_request().
Fixes: 95e34bf930ea ("scsi: ufs-bsg: Add support for raw upiu in ufs_bsg_request()")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
drivers/ufs/core/ufs_bsg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 18c318d78122..545ef7b79ceb 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -140,7 +140,7 @@ static int ufs_bsg_request(struct bsg_job *job)
int ret;
bool rpmb = false;
- bsg_reply->reply_payload_rcv_len = 0;
+ memset(job->reply, 0, job->reply_len);
ufshcd_rpm_get_sync(hba);
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/5] scsi: ufs: bsg: Clear the reply buffer before use
2026-10-15 0:18 ` [PATCH v2 2/5] scsi: ufs: bsg: Clear the reply buffer before use Stanley Jhu
@ 2026-10-15 0:34 ` sashiko-bot
2026-10-15 0:41 ` Stanley Jhu
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-10-15 0:34 UTC (permalink / raw)
To: Stanley Jhu; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ufs: heap overflow in ufs_bsg_exec_advanced_rpmb_req()
--
commit 8509b25f617ae6bb6b7c915900b84d8957279281
Author: Stanley Jhu <stanleyjhu@google.com>
scsi: ufs: bsg: Clear the reply buffer before use
This commit clears the bsg job reply buffer upon entry to ufs_bsg_request()
to prevent leaking bytes from an earlier request on the same tag. It
replaces a targeted zeroing of the length field with a memset of the entire
buffer.
> diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
> index 18c318d781224..545ef7b79ceb5 100644
> --- a/drivers/ufs/core/ufs_bsg.c
> +++ b/drivers/ufs/core/ufs_bsg.c
> @@ -140,7 +140,7 @@ static int ufs_bsg_request(struct bsg_job *job)
> int ret;
> bool rpmb = false;
>
> - bsg_reply->reply_payload_rcv_len = 0;
> + memset(job->reply, 0, job->reply_len);
[Severity: High]
Does this regression still allow a heap buffer overflow when processing
Advanced RPMB requests?
While the memset safely clears the 96 bytes initialized by bsg_init_rq()
in block/bsg-lib.c, the subsequent handling in
ufs_bsg_exec_advanced_rpmb_req() in drivers/ufs/core/ufs_bsg.c casts this
96-byte buffer to a struct ufs_rpmb_reply, which is 104 bytes in size.
When ufshcd_advanced_rpmb_req_handler() in drivers/ufs/core/ufshcd.c
handles the response, it executes:
memcpy(rsp_ehs, ehs_data, ehs_len * 32);
Will this copy write 64 bytes starting at offset 40, exceeding the 96-byte
allocation by 8 bytes and corrupting out-of-bounds memory?
>
> ufshcd_rpm_get_sync(hba);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261015001855.3450573-1-stanleyjhu@google.com?part=2
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/5] scsi: ufs: bsg: Clear the reply buffer before use
2026-10-15 0:34 ` sashiko-bot
@ 2026-10-15 0:41 ` Stanley Jhu
0 siblings, 0 replies; 13+ messages in thread
From: Stanley Jhu @ 2026-10-15 0:41 UTC (permalink / raw)
To: Martin K. Petersen, James E.J. Bottomley
Cc: Alim Akhtar, Bart Van Assche, Bean Huo, Peter Wang, Brian Kao,
linux-scsi, linux-kernel
On Thu, Oct 15, 2026 at 12:34:36AM +0000, sashiko-bot@kernel.org wrote:
> Will this copy write 64 bytes starting at offset 40, exceeding the 96-byte
> allocation by 8 bytes and corrupting out-of-bounds memory?
Pre-existing; fixed separately in bsg-lib, which sizes the reply buffer
to 104 bytes for UFS:
https://lore.kernel.org/all/20261013132708.1669415-1-stanleyjhu@google.com/
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/5] scsi: ufs: bsg: Validate request_len before accessing request
2026-10-15 0:18 [PATCH v2 0/5] scsi: ufs: Fix UIC result byte and BSG request handling Stanley Jhu
2026-10-15 0:18 ` [PATCH v2 1/5] scsi: ufs: bsg: Complete the job on error Stanley Jhu
2026-10-15 0:18 ` [PATCH v2 2/5] scsi: ufs: bsg: Clear the reply buffer before use Stanley Jhu
@ 2026-10-15 0:18 ` Stanley Jhu
2026-10-15 0:18 ` [PATCH v2 4/5] scsi: ufs: bsg: Reject UIC commands with a non-zero result byte Stanley Jhu
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Stanley Jhu @ 2026-10-15 0:18 UTC (permalink / raw)
To: Martin K. Petersen, James E.J. Bottomley
Cc: Alim Akhtar, Bart Van Assche, Bean Huo, Peter Wang, Brian Kao,
linux-scsi, linux-kernel, stable
ufs_bsg_request() and ufs_bsg_exec_advanced_rpmb_req() read the
request buffer without checking job->request_len, which userspace
controls. A short request causes an out-of-bounds read. Found by
syzkaller.
Reject requests shorter than the structure each path reads.
Fixes: 64d4864714c2 ("scsi: ufs: bsg: Remove unnecessary length checkup")
Fixes: 6ff265fc5ef6 ("scsi: ufs: core: bsg: Add advanced RPMB support in ufs_bsg")
Cc: stable@vger.kernel.org
Reviewed-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
Changes in v2:
- Complete the job on the early -EINVAL return through out_complete
(added in patch 1). Kept Bean's Reviewed-by; Bean, please let me
know if you would like to look again.
drivers/ufs/core/ufs_bsg.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 545ef7b79ceb..0db830e54ef0 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -73,6 +73,9 @@ static int ufs_bsg_exec_advanced_rpmb_req(struct ufs_hba *hba, struct bsg_job *j
if (hba->ufs_version < ufshci_version(4, 0) || !hba->dev_info.b_advanced_rpmb_en)
return -EINVAL;
+ if (job->request_len < sizeof(*rpmb_request))
+ return -EINVAL;
+
if (rpmb_request->ehs_req.length != 2 || rpmb_request->ehs_req.ehs_type != 1)
return -EINVAL;
@@ -142,6 +145,11 @@ static int ufs_bsg_request(struct bsg_job *job)
memset(job->reply, 0, job->reply_len);
+ if (job->request_len < sizeof(*bsg_request)) {
+ ret = -EINVAL;
+ goto out_complete;
+ }
+
ufshcd_rpm_get_sync(hba);
msgcode = bsg_request->msgcode;
@@ -194,6 +202,7 @@ static int ufs_bsg_request(struct bsg_job *job)
out:
ufshcd_rpm_put_sync(hba);
kfree(buff);
+out_complete:
bsg_reply->result = ret;
if (ret == 0)
job->reply_len = rpmb ? sizeof(struct ufs_rpmb_reply) :
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 4/5] scsi: ufs: bsg: Reject UIC commands with a non-zero result byte
2026-10-15 0:18 [PATCH v2 0/5] scsi: ufs: Fix UIC result byte and BSG request handling Stanley Jhu
` (2 preceding siblings ...)
2026-10-15 0:18 ` [PATCH v2 3/5] scsi: ufs: bsg: Validate request_len before accessing request Stanley Jhu
@ 2026-10-15 0:18 ` Stanley Jhu
2026-10-15 0:18 ` [PATCH v2 5/5] scsi: ufs: core: Clear stale UIC result byte before peer DME retries Stanley Jhu
2026-10-15 4:39 ` [PATCH v2 0/5] scsi: ufs: Fix UIC result byte and BSG request handling Peter Wang
5 siblings, 0 replies; 13+ messages in thread
From: Stanley Jhu @ 2026-10-15 0:18 UTC (permalink / raw)
To: Martin K. Petersen, James E.J. Bottomley
Cc: Alim Akhtar, Bart Van Assche, Bean Huo, Peter Wang, Brian Kao,
linux-scsi, linux-kernel, stable
ufs_bsg_request() passes the user-supplied UIC argument2 unchanged,
including the result byte (bits 7:0) that only the host controller
writes. A non-zero value triggers the WARN_ON_ONCE() in
ufshcd_dispatch_uic_cmd() and corrupts the reported result. Found by
syzkaller.
Reject UIC commands that have the result byte set with -EINVAL.
Fixes: e77044c5a842 ("scsi: ufs-bsg: Add support for uic commands in ufs_bsg_request()")
Fixes: 9fb4c793223b ("scsi: ufs: core: Complain if UIC argument 2 is invalid")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
Changes in v2:
- Reject a non-zero result byte with -EINVAL instead of clearing it,
as suggested by Bart Van Assche. Dropped Peter's and Bean's
Reviewed-by as the behavior changed.
drivers/ufs/core/ufs_bsg.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 0db830e54ef0..cadde515f040 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -179,6 +179,10 @@ static int ufs_bsg_request(struct bsg_job *job)
break;
case UPIU_TRANSACTION_UIC_CMD:
memcpy(&uc, &bsg_request->upiu_req.uc, UIC_CMD_SIZE);
+ if (uc.argument2 & MASK_UIC_COMMAND_RESULT) {
+ ret = -EINVAL;
+ break;
+ }
ret = ufshcd_send_bsg_uic_cmd(hba, &uc);
if (ret)
dev_err(hba->dev, "send uic cmd: error code %d\n", ret);
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 5/5] scsi: ufs: core: Clear stale UIC result byte before peer DME retries
2026-10-15 0:18 [PATCH v2 0/5] scsi: ufs: Fix UIC result byte and BSG request handling Stanley Jhu
` (3 preceding siblings ...)
2026-10-15 0:18 ` [PATCH v2 4/5] scsi: ufs: bsg: Reject UIC commands with a non-zero result byte Stanley Jhu
@ 2026-10-15 0:18 ` Stanley Jhu
2026-10-15 0:32 ` sashiko-bot
2026-10-15 4:39 ` [PATCH v2 0/5] scsi: ufs: Fix UIC result byte and BSG request handling Peter Wang
5 siblings, 1 reply; 13+ messages in thread
From: Stanley Jhu @ 2026-10-15 0:18 UTC (permalink / raw)
To: Martin K. Petersen, James E.J. Bottomley
Cc: Alim Akhtar, Bart Van Assche, Bean Huo, Peter Wang, Brian Kao,
linux-scsi, linux-kernel, stable
Peer DME get and set retry a failed attempt with the same struct
uic_command. UIC completion ORs the ConfigResultCode into
argument2[7:0], and nothing clears it before the next attempt. Once an
attempt fails, every retry keeps reporting that error code, even when
the device returns success, so a transient failure becomes a hard
failure after three retries. Each such retry also triggers the
WARN_ON_ONCE() in ufshcd_dispatch_uic_cmd(). In ufshcd_dme_set_attr(),
ufshcd_uic_cmd_compl() also overwrites argument3 with the register
readback before the next attempt.
Clear the result byte and restore argument3 in ufshcd_dme_set_attr()
when an attempt fails.
Fixes: 64238fbd15ea ("scsi: ufs: add retries to dme_peer get and set attribute")
Cc: stable@vger.kernel.org
Reviewed-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
Changes in v2:
- Clear argument2's result byte and restore argument3 (MIBvalue_W) in
the failure path instead of before the first attempt. Kept Bean's
Reviewed-by; Bean, please let me know if you would like to look again.
drivers/ufs/core/ufshcd.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index bcc2512bf34b..7532fd916bfd 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4400,9 +4400,12 @@ int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
do {
/* for peer attributes we retry upon failure */
ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
- if (ret)
+ if (ret) {
dev_dbg(hba->dev, "%s: attr-id 0x%x val 0x%x error code %d\n",
set, UIC_GET_ATTR_ID(attr_sel), mib_val, ret);
+ uic_cmd.argument2 &= ~MASK_UIC_COMMAND_RESULT;
+ uic_cmd.argument3 = mib_val;
+ }
} while (ret && peer && --retries);
if (ret)
@@ -4467,9 +4470,11 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
do {
/* for peer attributes we retry upon failure */
ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
- if (ret)
+ if (ret) {
dev_dbg(hba->dev, "%s: attr-id 0x%x error code %d\n",
get, UIC_GET_ATTR_ID(attr_sel), ret);
+ uic_cmd.argument2 &= ~MASK_UIC_COMMAND_RESULT;
+ }
} while (ret && peer && --retries);
if (ret)
--
2.56.0.385.gd3acb90ef8-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 5/5] scsi: ufs: core: Clear stale UIC result byte before peer DME retries
2026-10-15 0:18 ` [PATCH v2 5/5] scsi: ufs: core: Clear stale UIC result byte before peer DME retries Stanley Jhu
@ 2026-10-15 0:32 ` sashiko-bot
2026-10-15 0:43 ` Stanley Jhu
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-10-15 0:32 UTC (permalink / raw)
To: Stanley Jhu; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ufs: race in ufshcd_dispatch_uic_cmd() between retry and late interrupt
--
commit 2dc892f166dff54aa2e190ad9b52806532df339b
Author: Stanley Jhu <stanleyjhu@google.com>
scsi: ufs: core: Clear stale UIC result byte before peer DME retries
This commit clears the stale UIC result byte and restores argument3 when an
attempt fails before peer DME retries. This prevents a transient failure from
becoming a hard failure after retries due to uncleared error codes.
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index bcc2512bf34b8..7532fd916bfd9 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -4400,9 +4400,12 @@ int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
> do {
> /* for peer attributes we retry upon failure */
> ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
> - if (ret)
> + if (ret) {
> dev_dbg(hba->dev, "%s: attr-id 0x%x val 0x%x error code %d\n",
> set, UIC_GET_ATTR_ID(attr_sel), mib_val, ret);
> + uic_cmd.argument2 &= ~MASK_UIC_COMMAND_RESULT;
> + uic_cmd.argument3 = mib_val;
> + }
> } while (ret && peer && --retries);
[Severity: High]
Can a retried UIC command be prematurely completed by a late interrupt from a
previous timed-out command?
If a call to ufshcd_send_uic_cmd() times out, the retry loop immediately
invokes it again. In ufshcd_dispatch_uic_cmd(), the new command is assigned
to hba->active_uic_cmd without holding the host_lock:
drivers/ufs/core/ufshcd.c:ufshcd_dispatch_uic_cmd() {
...
hba->active_uic_cmd = uic_cmd;
...
}
If a late interrupt from the previously timed-out command fires right after
this assignment, the ISR will read the new command under host_lock and
complete it prematurely:
drivers/ufs/core/ufshcd.c:ufshcd_uic_cmd_compl() {
...
guard(spinlock_irqsave)(&hba->host->host_lock);
cmd = hba->active_uic_cmd;
if (!cmd) {
...
}
This could lead to invalid configuration and potential controller
desynchronization, since the new command is completed using the old command's
result.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261015001855.3450573-1-stanleyjhu@google.com?part=5
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 5/5] scsi: ufs: core: Clear stale UIC result byte before peer DME retries
2026-10-15 0:32 ` sashiko-bot
@ 2026-10-15 0:43 ` Stanley Jhu
0 siblings, 0 replies; 13+ messages in thread
From: Stanley Jhu @ 2026-10-15 0:43 UTC (permalink / raw)
To: Martin K. Petersen, James E.J. Bottomley
Cc: Alim Akhtar, Bart Van Assche, Bean Huo, Peter Wang, Brian Kao,
linux-scsi, linux-kernel
On Thu, Oct 15, 2026 at 12:32:04AM +0000, sashiko-bot@kernel.org wrote:
> Can a retried UIC command be prematurely completed by a late interrupt from a
> previous timed-out command?
This patch does not change that path. The retry loop and the UIC
dispatch/completion handling are the same as before; it only resets
argument2 and argument3 between attempts.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/5] scsi: ufs: Fix UIC result byte and BSG request handling
2026-10-15 0:18 [PATCH v2 0/5] scsi: ufs: Fix UIC result byte and BSG request handling Stanley Jhu
` (4 preceding siblings ...)
2026-10-15 0:18 ` [PATCH v2 5/5] scsi: ufs: core: Clear stale UIC result byte before peer DME retries Stanley Jhu
@ 2026-10-15 4:39 ` Peter Wang
5 siblings, 0 replies; 13+ messages in thread
From: Peter Wang @ 2026-10-15 4:39 UTC (permalink / raw)
To: Stanley Jhu, Martin K. Petersen, James E.J. Bottomley
Cc: Alim Akhtar, Bart Van Assche, Bean Huo, Brian Kao, linux-scsi,
linux-kernel
On Thu, 2026-10-15 at 08:18 +0800, Stanley Jhu wrote:
> ufs-bsg fixes for issues found by fuzzing /dev/bsg/ufs-bsg* with
> syzkaller on QEMU's emulated UFS host controller and in review of v1,
> plus a fix for peer DME retries.
>
> Patch 1 completes failed BSG requests via bsg_job_done() so error
> paths
> do not leak the job's device reference and mapped scatterlists.
> Patch 2 clears the reused reply buffer so replies do not carry bytes
> from an earlier request.
> Patch 3 rejects requests shorter than the UFS BSG request structures.
> Patch 4 rejects UIC commands with a non-zero result byte from
> userspace.
> Patch 5 clears the stale UIC result byte and restores argument3
> before
> peer DME retries.
>
> Patches 1-2 also address the sashiko review of the bsg reply_len
> patch:
> https://lore.kernel.org/all/sashiko-outbox-169843@kernel.org/
>
> Tested on QEMU arm64: the reproducers no longer trigger.
>
> Differences from v1:
> - New patch 1/5: complete failed jobs via bsg_job_done().
> - New patch 2/5: clear the reply buffer on entry.
> - Patch 4/5: reject non-zero UIC result byte with -EINVAL (Bart).
> - Patch 5/5: restore argument3 on each retry in
> ufshcd_dme_set_attr().
>
> Link:
> https://lore.kernel.org/all/20261013102913.948718-1-stanleyjhu@google.com/
>
>
> Stanley Jhu (5):
> scsi: ufs: bsg: Complete the job on error
> scsi: ufs: bsg: Clear the reply buffer before use
> scsi: ufs: bsg: Validate request_len before accessing request
> scsi: ufs: bsg: Reject UIC commands with a non-zero result byte
> scsi: ufs: core: Clear stale UIC result byte before peer DME
> retries
>
> drivers/ufs/core/ufs_bsg.c | 23 +++++++++++++++++------
> drivers/ufs/core/ufshcd.c | 9 +++++++--
> 2 files changed, 24 insertions(+), 8 deletions(-)
>
> base-commit: 9621cbd332862eb6651e2aba24ed18f66560de66
For the series:
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
^ permalink raw reply [flat|nested] 13+ messages in thread