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

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

[Stanley Jhu]

Two ufs-bsg fixes for issues found by fuzzing /dev/bsg/ufs-bsg* with
syzkaller on QEMU's emulated UFS host controller, plus a related fix
for the same UIC result byte problem in the peer DME retry path.

Patch 1 rejects requests shorter than the UFS BSG request structures.
Patch 2 clears the UIC result byte supplied by userspace.
Patch 3 clears the stale UIC result byte before peer DME retries.

Tested on QEMU arm64: the reproducers no longer trigger.

Stanley Jhu (3):
  scsi: ufs: bsg: Validate request_len before accessing request
  scsi: ufs: bsg: Clear the UIC result byte before sending UIC commands
  scsi: ufs: core: Clear stale UIC result byte before peer DME retries

 drivers/ufs/core/ufs_bsg.c | 9 +++++++++
 drivers/ufs/core/ufshcd.c  | 2 ++
 2 files changed, 11 insertions(+)

-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH 1/3] 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
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufs_bsg.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 7a66917b3fc0..dab1f05aff38 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)
 
 	bsg_reply->reply_payload_rcv_len = 0;
 
+	if (job->request_len < sizeof(*bsg_request)) {
+		bsg_reply->result = -EINVAL;
+		return -EINVAL;
+	}
+
 	ufshcd_rpm_get_sync(hba);
 
 	msgcode = bsg_request->msgcode;
-- 
2.56.0.385.gd3acb90ef8-goog

Re: [PATCH 1/3] scsi: ufs: bsg: Validate request_len before accessing request

[Bean Huo]

On Tue, 2026-10-13 at 18:29 +0800, Stanley Jhu wrote:
> 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
> Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
> ---
>  drivers/ufs/core/ufs_bsg.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
> index 7a66917b3fc0..dab1f05aff38 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)
>  
>         bsg_reply->reply_payload_rcv_len = 0;
>  
> +       if (job->request_len < sizeof(*let )) {
> +               bsg_reply->result = -EINVAL;
> +               return -EINVAL;
> +       }
> +
>         ufshcd_rpm_get_sync(hba);
>  
>         msgcode = bsg_request->msgcode;

Stanley, 

The check was removed by my commit 64d4864714c2 when adding arpmb support, since
msgcode check was thought to be enough, thanks for fixing it. 


Reviewed-by: Bean Huo <beanhuo@micron.com>

[PATCH 2/3] scsi: ufs: bsg: Clear the UIC result byte before sending UIC commands

[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.

Clear the result byte before sending the command.

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>
---
 drivers/ufs/core/ufs_bsg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index dab1f05aff38..331ead0403a1 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -179,6 +179,7 @@ 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);
+		uc.argument2 &= ~MASK_UIC_COMMAND_RESULT;
 		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

Re: [PATCH 2/3] scsi: ufs: bsg: Clear the UIC result byte before sending UIC commands

[Peter Wang]

On Tue, 2026-10-13 at 18:29 +0800, Stanley Jhu wrote:
> 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.
> 
> Clear the result byte before sending the command.
> 
> 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>
> ---


Reviewed-by: Peter Wang <peter.wang@mediatek.com>

Re: [PATCH 2/3] scsi: ufs: bsg: Clear the UIC result byte before sending UIC commands

[Bean Huo]

On Tue, 2026-10-13 at 18:29 +0800, Stanley Jhu wrote:
> 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.
> 
> Clear the result byte before sending the command.
> 
> 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>

Reviewed-by: Bean Huo <beanhuo@micron.com>

Re: [PATCH 2/3] scsi: ufs: bsg: Clear the UIC result byte before sending UIC commands

[Bart Van Assche]

On 10/13/26 3:29 AM, Stanley Jhu wrote:
> 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.
> 
> Clear the result byte before sending the command.
> 
> 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>
> ---
>   drivers/ufs/core/ufs_bsg.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
> index dab1f05aff38..331ead0403a1 100644
> --- a/drivers/ufs/core/ufs_bsg.c
> +++ b/drivers/ufs/core/ufs_bsg.c
> @@ -179,6 +179,7 @@ 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);
> +		uc.argument2 &= ~MASK_UIC_COMMAND_RESULT;
>   		ret = ufshcd_send_bsg_uic_cmd(hba, &uc);
>   		if (ret)
>   			dev_err(hba->dev, "send uic cmd: error code %d\n", ret);

Shouldn't invalid inputs be rejected rather than trying to correct these?

Thanks,

Bart.

[PATCH 3/3] 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().

Clear the result byte before each attempt.

Fixes: 64238fbd15ea ("scsi: ufs: add retries to dme_peer get and set attribute")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index bcc2512bf34b..9adb3b576e28 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -4398,6 +4398,7 @@ int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
 	int retries = UFS_UIC_COMMAND_RETRIES;
 
 	do {
+		uic_cmd.argument2 &= ~MASK_UIC_COMMAND_RESULT;
 		/* for peer attributes we retry upon failure */
 		ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
 		if (ret)
@@ -4465,6 +4466,7 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
 	}
 
 	do {
+		uic_cmd.argument2 &= ~MASK_UIC_COMMAND_RESULT;
 		/* for peer attributes we retry upon failure */
 		ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
 		if (ret)
-- 
2.56.0.385.gd3acb90ef8-goog

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

[Bean Huo]

On Tue, 2026-10-13 at 18:29 +0800, Stanley Jhu wrote:
> 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().
> 
> Clear the result byte before each attempt.
> 
> Fixes: 64238fbd15ea ("scsi: ufs: add retries to dme_peer get and set
> attribute")
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
> ---
>  drivers/ufs/core/ufshcd.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index bcc2512bf34b..9adb3b576e28 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -4398,6 +4398,7 @@ int ufshcd_dme_set_attr(struct ufs_hba *hba, u32
> attr_sel,
>         int retries = UFS_UIC_COMMAND_RETRIES;
>  
>         do {
> +               uic_cmd.argument2 &= ~MASK_UIC_COMMAND_RESULT;
>                 /* for peer attributes we retry upon failure */
>                 ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
>                 if (ret)
> @@ -4465,6 +4466,7 @@ int ufshcd_dme_get_attr(struct ufs_hba *hba, u32
> attr_sel,
>         }
>  
>         do {
> +               uic_cmd.argument2 &= ~MASK_UIC_COMMAND_RESULT;
>                 /* for peer attributes we retry upon failure */
>                 ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
>                 if (ret)

Stanley,

AI Sashiko comment on arg3 should not happen on a spec compliant ufshci
controller, since arg3 is an input for dme_set, arg2 indeed has an output byte
for every command, but restoring arg3 in the set loop would be cheap safety
enhancement, but up to you:

for me this patch is ok:

Reviewed-by: Bean Huo <beanhuo@micron.com>