[PATCH v2 0/5] scsi: ufs: Fix UIC result byte and BSG request handling

[PATCH v2 0/5] scsi: ufs: Fix UIC result byte and BSG request handling

[Stanley Jhu]

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
-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH v2 1/5] scsi: ufs: bsg: Complete the job on error

[Stanley Jhu]

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

[PATCH v2 2/5] scsi: ufs: bsg: Clear the reply buffer before use

[Stanley Jhu]

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

[PATCH v2 3/5] scsi: ufs: bsg: Validate request_len before accessing request

[Stanley Jhu]

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

[PATCH v2 4/5] scsi: ufs: bsg: Reject UIC commands with a non-zero result byte

[Stanley Jhu]

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

[PATCH v2 5/5] scsi: ufs: core: Clear stale UIC result byte before peer DME retries

[Stanley Jhu]

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

Re: [PATCH v2 0/5] scsi: ufs: Fix UIC result byte and BSG request handling

[Peter Wang]

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>

Re: [PATCH v2 2/5] scsi: ufs: bsg: Clear the reply buffer before use

[Stanley Jhu]

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/