[PATCH v2] ufs: core: bsg: Fix memory crash in case arpmb command failed

[PATCH v2] ufs: core: bsg: Fix memory crash in case arpmb command failed

[Arthur Simchaev]

In case the device doesn't support arpmb, the kernel get memory crash
due to copy user data in bsg_transport_sg_io_fn level. So in case
ufs_bsg_exec_advanced_rpmb_req returned error, do not set the job's
reply_len.

Memory crash backtrace:
3,1290,531166405,-;ufshcd 0000:00:12.5: ARPMB OP failed: error code -22

4,1308,531166555,-;Call Trace:

4,1309,531166559,-; <TASK>

4,1310,531166565,-; ? show_regs+0x6d/0x80

4,1311,531166575,-; ? die+0x37/0xa0

4,1312,531166583,-; ? do_trap+0xd4/0xf0

4,1313,531166593,-; ? do_error_trap+0x71/0xb0

4,1314,531166601,-; ? usercopy_abort+0x6c/0x80

4,1315,531166610,-; ? exc_invalid_op+0x52/0x80

4,1316,531166622,-; ? usercopy_abort+0x6c/0x80

4,1317,531166630,-; ? asm_exc_invalid_op+0x1b/0x20

4,1318,531166643,-; ? usercopy_abort+0x6c/0x80

4,1319,531166652,-; __check_heap_object+0xe3/0x120

4,1320,531166661,-; check_heap_object+0x185/0x1d0

4,1321,531166670,-; __check_object_size.part.0+0x72/0x150

4,1322,531166679,-; __check_object_size+0x23/0x30

4,1323,531166688,-; bsg_transport_sg_io_fn+0x314/0x3b0

Fixes: 6ff265fc5ef6 ("scsi: ufs: core: bsg: Add advanced RPMB support in ufs_bsg")
Cc: stable@vger.kernel.org
Signed-off-by: Arthur Simchaev <arthur.simchaev@sandisk.com>

---
Changes in v2:
  - Add Fixes tag
  - Elaborate commit log

Signed-off-by: Arthur Simchaev <arthur.simchaev@sandisk.com>
---
 drivers/ufs/core/ufs_bsg.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 8d4ad0a3f2cf..a8ed9bc6e4f1 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -194,10 +194,12 @@ static int ufs_bsg_request(struct bsg_job *job)
 	ufshcd_rpm_put_sync(hba);
 	kfree(buff);
 	bsg_reply->result = ret;
-	job->reply_len = !rpmb ? sizeof(struct ufs_bsg_reply) : sizeof(struct ufs_rpmb_reply);
 	/* complete the job here only if no error */
-	if (ret == 0)
+	if (ret == 0) {
+		job->reply_len = !rpmb ? sizeof(struct ufs_bsg_reply) :
+					 sizeof(struct ufs_rpmb_reply);
 		bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
+	}
 
 	return ret;
 }
-- 
2.34.1

Re: [PATCH v2] ufs: core: bsg: Fix memory crash in case arpmb command failed

[Bean Huo]

On Tue, 2025-02-18 at 13:15 +0200, Arthur Simchaev wrote:
> In case the device doesn't support arpmb, the kernel get memory crash
> due to copy user data in bsg_transport_sg_io_fn level. So in case
> ufs_bsg_exec_advanced_rpmb_req returned error, do not set the job's
> reply_len.
> 
> Memory crash backtrace:
> 3,1290,531166405,-;ufshcd 0000:00:12.5: ARPMB OP failed: error code -
> 22
> 
> 4,1308,531166555,-;Call Trace:
> 
> 4,1309,531166559,-; <TASK>
> 
> 4,1310,531166565,-; ? show_regs+0x6d/0x80
> 
> 4,1311,531166575,-; ? die+0x37/0xa0
> 
> 4,1312,531166583,-; ? do_trap+0xd4/0xf0
> 
> 4,1313,531166593,-; ? do_error_trap+0x71/0xb0
> 
> 4,1314,531166601,-; ? usercopy_abort+0x6c/0x80
> 
> 4,1315,531166610,-; ? exc_invalid_op+0x52/0x80
> 
> 4,1316,531166622,-; ? usercopy_abort+0x6c/0x80
> 
> 4,1317,531166630,-; ? asm_exc_invalid_op+0x1b/0x20
> 
> 4,1318,531166643,-; ? usercopy_abort+0x6c/0x80
> 
> 4,1319,531166652,-; __check_heap_object+0xe3/0x120
> 
> 4,1320,531166661,-; check_heap_object+0x185/0x1d0
> 
> 4,1321,531166670,-; __check_object_size.part.0+0x72/0x150
> 
> 4,1322,531166679,-; __check_object_size+0x23/0x30
> 
> 4,1323,531166688,-; bsg_transport_sg_io_fn+0x314/0x3b0
> 
> Fixes: 6ff265fc5ef6 ("scsi: ufs: core: bsg: Add advanced RPMB support
> in ufs_bsg")
> Cc: stable@vger.kernel.org
> Signed-off-by: Arthur Simchaev <arthur.simchaev@sandisk.com>
> 
> ---
> Changes in v2:
>   - Add Fixes tag
>   - Elaborate commit log
> 
> Signed-off-by: Arthur Simchaev <arthur.simchaev@sandisk.com>
> ---
>  drivers/ufs/core/ufs_bsg.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
> index 8d4ad0a3f2cf..a8ed9bc6e4f1 100644
> --- a/drivers/ufs/core/ufs_bsg.c
> +++ b/drivers/ufs/core/ufs_bsg.c
> @@ -194,10 +194,12 @@ static int ufs_bsg_request(struct bsg_job *job)
>         ufshcd_rpm_put_sync(hba);
>         kfree(buff);
>         bsg_reply->result = ret;
> -       job->reply_len = !rpmb ? sizeof(struct ufs_bsg_reply) :
> sizeof(struct ufs_rpmb_reply);
>         /* complete the job here only if no error */
> -       if (ret == 0)
> +       if (ret == 0) {
> +               job->reply_len = !rpmb ? sizeof(struct ufs_bsg_reply)
> :
> +                                        sizeof(struct
> ufs_rpmb_reply);
>                 bsg_job_done(job, ret, bsg_reply-
> >reply_payload_rcv_len);
> +       }
>  
>         return ret;
>  }


Arthur,

thanks for your update. 

I tried to repoduce the issue as your steps, I didn't get this issue,
The kernel will only print this as expected: 

Err: ARPMB OP failed 0 :-22



I don't think your patch can fix your issue, becase if ufs_bsg returns 

-EINVAL(-22).  then, 


bsg_reply->result = ret(-22);

after that,  then in bsg_transport_sg_io_fn:

if (job->result < 0) {
	job->reply_len = sizeof(u32);  //overwrite the length.



Could you please provide more information how you can get this issue?
My understanding is that it is not because this job->reply_len, it is
your buffer initiated by your application?


Kind regards,
Bean

RE: [PATCH v2] ufs: core: bsg: Fix memory crash in case arpmb command failed

[Arthur Simchaev]

Hi Bean

The issue 100 % reproducible on the platform where the UFS device is secondary device. 
Device is UFS 4.0 configured to support RPMB.
I am using ufs-utils tool with your committed arpmb code.  For example. run get write arpmb counter command:
./ufs-utils arpmb -t 1 -p /dev/ufs-bsg. 
After the change, the crash doesn't occur.  See the full kernel crash before the fix:
Let me know if you need more details

3,1290,531166405,-;ufshcd 0000:00:12.5: ARPMB OP failed: error code -22

SUBSYSTEM=pci

DEVICE=+pci:0000:00:12.5

0,1291,531166433,-;usercopy: Kernel memory exposure attempt detected from SLUB object 'kmalloc-96' (offset 0, size 104)!

4,1292,531166452,-;------------[ cut here ]------------

2,1293,531166455,-;kernel BUG at mm/usercopy.c:102!

4,1294,531166467,-;invalid opcode: 0000 [#1] PREEMPT SMP NOPTI

4,1295,531166475,-;CPU: 4 PID: 3321 Comm: ufs-utils-micro Not tainted 6.4.0-060400-generic #202306271339

4,1296,531166483,-;Hardware name: SAMSUNG ELECTRONICS CO., LTD. 767XCL/NT767XCL-KLTES, BIOS P07AJD.053.200820.KS 08/20/2020

4,1297,531166487,-;RIP: 0010:usercopy_abort+0x6c/0x80

4,1298,531166504,-;Code: 75 86 51 48 c7 c2 4f a3 7a 86 41 52 48 c7 c7 38 1f 77 86 48 0f 45 d6 48 c7 c6 fb 2c 75 86 48 89 c1 49 0f 45 f3 e8 c4 9e d0 ff <0f> 0b 49 c7 c1 b8 e1 74 86 4d 89 ca 4d 89 c8 eb a8 0f 1f 00 90 90

4,1299,531166511,-;RSP: 0018:ffffb1d2c217bc10 EFLAGS: 00010246

4,1300,531166520,-;RAX: 0000000000000065 RBX: 0000000000000000 RCX: 0000000000000000

4,1301,531166524,-;RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000

4,1302,531166528,-;RBP: ffffb1d2c217bc28 R08: 0000000000000000 R09: 0000000000000000

4,1303,531166531,-;R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000068

4,1304,531166535,-;R13: ffff911d40042600 R14: 0000000000000001 R15: 00007ffe9126ede0

4,1305,531166539,-;FS: 000000000071b3c0(0000) GS:ffff911ea7600000(0000) knlGS:0000000000000000

4,1306,531166545,-;CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033

4,1307,531166550,-;CR2: 00007ffe9126eff8 CR3: 00000001856e0000 CR4: 0000000000350ee0

4,1308,531166555,-;Call Trace:

4,1309,531166559,-; <TASK>

4,1310,531166565,-; ? show_regs+0x6d/0x80

4,1311,531166575,-; ? die+0x37/0xa0

4,1312,531166583,-; ? do_trap+0xd4/0xf0

4,1313,531166593,-; ? do_error_trap+0x71/0xb0

4,1314,531166601,-; ? usercopy_abort+0x6c/0x80

4,1315,531166610,-; ? exc_invalid_op+0x52/0x80

4,1316,531166622,-; ? usercopy_abort+0x6c/0x80

4,1317,531166630,-; ? asm_exc_invalid_op+0x1b/0x20

4,1318,531166643,-; ? usercopy_abort+0x6c/0x80

4,1319,531166652,-; __check_heap_object+0xe3/0x120

4,1320,531166661,-; check_heap_object+0x185/0x1d0

4,1321,531166670,-; __check_object_size.part.0+0x72/0x150

4,1322,531166679,-; __check_object_size+0x23/0x30

4,1323,531166688,-; bsg_transport_sg_io_fn+0x314/0x3b0

4,1324,531166699,-; ? __pfx_bsg_transport_sg_io_fn+0x10/0x10

4,1325,531166707,-; bsg_sg_io+0x9e/0x120

4,1326,531166717,-; bsg_ioctl+0x1f4/0x240

4,1327,531166723,-; __x64_sys_ioctl+0x9d/0xe0

4,1328,531166734,-; do_syscall_64+0x58/0x90

4,1329,531166743,-; ? putname+0x5d/0x80

4,1330,531166752,-; ? do_sys_openat2+0xab/0x180

4,1331,531166761,-; ? exit_to_user_mode_prepare+0x30/0xb0

4,1332,531166771,-; ? syscall_exit_to_user_mode+0x29/0x50

4,1333,531166781,-; ? do_syscall_64+0x67/0x90

4,1334,531166788,-; ? irqentry_exit_to_user_mode+0x9/0x20

4,1335,531166798,-; ? irqentry_exit+0x43/0x50

4,1336,531166806,-; ? exc_page_fault+0x94/0x1b0

4,1337,531166815,-; entry_SYSCALL_64_after_hwframe+0x72/0xdc

4,1338,531166824,-;RIP: 0033:0x45759f

4,1339,531166871,-;Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <41> 89 c0 3d 00 f0 ff ff 77 1f 48 8b 44 24 18 64 48 2b 04 25 28 00

4,1340,531166877,-;RSP: 002b:00007ffe9126eba0 EFLAGS: 00000246 ORIG_RAX: 0000000000000010



-----Original Message-----
From: Bean Huo <huobean@gmail.com> 
Sent: Tuesday, February 18, 2025 2:31 PM
To: Arthur Simchaev <Arthur.Simchaev@sandisk.com>; martin.petersen@oracle.com
Cc: Avri Altman <Avri.Altman@sandisk.com>; Avi Shchislowski <Avi.Shchislowski@sandisk.com>; beanhuo@micron.com; linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; bvanassche@acm.org; stable@vger.kernel.org
Subject: Re: [PATCH v2] ufs: core: bsg: Fix memory crash in case arpmb command failed

On Tue, 2025-02-18 at 13:15 +0200, Arthur Simchaev wrote:
> In case the device doesn't support arpmb, the kernel get memory crash 
> due to copy user data in bsg_transport_sg_io_fn level. So in case 
> ufs_bsg_exec_advanced_rpmb_req returned error, do not set the job's 
> reply_len.
> 
> Memory crash backtrace:
> 3,1290,531166405,-;ufshcd 0000:00:12.5: ARPMB OP failed: error code -
> 22
> 
> 4,1308,531166555,-;Call Trace:
> 
> 4,1309,531166559,-; <TASK>
> 
> 4,1310,531166565,-; ? show_regs+0x6d/0x80
> 
> 4,1311,531166575,-; ? die+0x37/0xa0
> 
> 4,1312,531166583,-; ? do_trap+0xd4/0xf0
> 
> 4,1313,531166593,-; ? do_error_trap+0x71/0xb0
> 
> 4,1314,531166601,-; ? usercopy_abort+0x6c/0x80
> 
> 4,1315,531166610,-; ? exc_invalid_op+0x52/0x80
> 
> 4,1316,531166622,-; ? usercopy_abort+0x6c/0x80
> 
> 4,1317,531166630,-; ? asm_exc_invalid_op+0x1b/0x20
> 
> 4,1318,531166643,-; ? usercopy_abort+0x6c/0x80
> 
> 4,1319,531166652,-; __check_heap_object+0xe3/0x120
> 
> 4,1320,531166661,-; check_heap_object+0x185/0x1d0
> 
> 4,1321,531166670,-; __check_object_size.part.0+0x72/0x150
> 
> 4,1322,531166679,-; __check_object_size+0x23/0x30
> 
> 4,1323,531166688,-; bsg_transport_sg_io_fn+0x314/0x3b0
> 
> Fixes: 6ff265fc5ef6 ("scsi: ufs: core: bsg: Add advanced RPMB support 
> in ufs_bsg")
> Cc: stable@vger.kernel.org
> Signed-off-by: Arthur Simchaev <arthur.simchaev@sandisk.com>
> 
> ---
> Changes in v2:
>   - Add Fixes tag
>   - Elaborate commit log
> 
> Signed-off-by: Arthur Simchaev <arthur.simchaev@sandisk.com>
> ---
>  drivers/ufs/core/ufs_bsg.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c 
> index 8d4ad0a3f2cf..a8ed9bc6e4f1 100644
> --- a/drivers/ufs/core/ufs_bsg.c
> +++ b/drivers/ufs/core/ufs_bsg.c
> @@ -194,10 +194,12 @@ static int ufs_bsg_request(struct bsg_job *job)
>         ufshcd_rpm_put_sync(hba);
>         kfree(buff);
>         bsg_reply->result = ret;
> -       job->reply_len = !rpmb ? sizeof(struct ufs_bsg_reply) :
> sizeof(struct ufs_rpmb_reply);
>         /* complete the job here only if no error */
> -       if (ret == 0)
> +       if (ret == 0) {
> +               job->reply_len = !rpmb ? sizeof(struct ufs_bsg_reply)
> :
> +                                        sizeof(struct
> ufs_rpmb_reply);
>                 bsg_job_done(job, ret, bsg_reply-
> >reply_payload_rcv_len);
> +       }
>  
>         return ret;
>  }


Arthur,

thanks for your update. 

I tried to repoduce the issue as your steps, I didn't get this issue, The kernel will only print this as expected: 

Err: ARPMB OP failed 0 :-22



I don't think your patch can fix your issue, becase if ufs_bsg returns 

-EINVAL(-22).  then, 


bsg_reply->result = ret(-22);

after that,  then in bsg_transport_sg_io_fn:

if (job->result < 0) {
	job->reply_len = sizeof(u32);  //overwrite the length.



Could you please provide more information how you can get this issue?
My understanding is that it is not because this job->reply_len, it is your buffer initiated by your application?


Kind regards,
Bean

Re: [PATCH v2] ufs: core: bsg: Fix memory crash in case arpmb command failed

[Bean Huo]

On Tue, 2025-02-18 at 13:08 +0000, Arthur Simchaev wrote:
> Hi Bean
> 
> The issue 100 % reproducible on the platform where the UFS device is
> secondary device. 
> Device is UFS 4.0 configured to support RPMB.
> I am using ufs-utils tool with your committed arpmb code.  For
> example. run get write arpmb counter command:
> ./ufs-utils arpmb -t 1 -p /dev/ufs-bsg. 
> After the change, the crash doesn't occur.  See the full kernel crash
> before the fix:
> Let me know if you need more details
> 
> 3,1290,531166405,-;ufshcd 0000:00:12.5: ARPMB OP failed: error code -
> 22
> 
> SUBSYSTEM=pci
> 
> DEVICE=+pci:0000:00:12.5
> 
> 0,1291,531166433,-;usercopy: Kernel memory exposure attempt detected
> from SLUB object 'kmalloc-96' (offset 0, size 104)!
> 
> 4,1292,531166452,-;------------[ cut here ]------------
> 
> 2,1293,531166455,-;kernel BUG at mm/usercopy.c:102!
> 
> 4,1294,531166467,-;invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
> 
> 4,1295,531166475,-;CPU: 4 PID: 3321 Comm: ufs-utils-micro Not tainted
> 6.4.0-060400-generic #202306271339
> 
> 4,1296,531166483,-;Hardware name: SAMSUNG ELECTRONICS CO., LTD.
> 767XCL/NT767XCL-KLTES, BIOS P07AJD.053.200820.KS 08/20/2020
> 
> 4,1297,531166487,-;RIP: 0010:usercopy_abort+0x6c/0x80
> 
> 4,1298,531166504,-;Code: 75 86 51 48 c7 c2 4f a3 7a 86 41 52 48 c7 c7
> 38 1f 77 86 48 0f 45 d6 48 c7 c6 fb 2c 75 86 48 89 c1 49 0f 45 f3 e8
> c4 9e d0 ff <0f> 0b 49 c7 c1 b8 e1 74 86 4d 89 ca 4d 89 c8 eb a8 0f
> 1f 00 90 90
> 
> 4,1299,531166511,-;RSP: 0018:ffffb1d2c217bc10 EFLAGS: 00010246
> 
> 4,1300,531166520,-;RAX: 0000000000000065 RBX: 0000000000000000 RCX:
> 0000000000000000
> 
> 4,1301,531166524,-;RDX: 0000000000000000 RSI: 0000000000000000 RDI:
> 0000000000000000
> 
> 4,1302,531166528,-;RBP: ffffb1d2c217bc28 R08: 0000000000000000 R09:
> 0000000000000000
> 
> 4,1303,531166531,-;R10: 0000000000000000 R11: 0000000000000000 R12:
> 0000000000000068
> 
> 4,1304,531166535,-;R13: ffff911d40042600 R14: 0000000000000001 R15:
> 00007ffe9126ede0
> 
> 4,1305,531166539,-;FS: 000000000071b3c0(0000)
> GS:ffff911ea7600000(0000) knlGS:0000000000000000
> 
> 4,1306,531166545,-;CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> 
> 4,1307,531166550,-;CR2: 00007ffe9126eff8 CR3: 00000001856e0000 CR4:
> 0000000000350ee0
> 
> 4,1308,531166555,-;Call Trace:
> 
> 4,1309,531166559,-; <TASK>
> 
> 4,1310,531166565,-; ? show_regs+0x6d/0x80
> 
> 4,1311,531166575,-; ? die+0x37/0xa0
> 
> 4,1312,531166583,-; ? do_trap+0xd4/0xf0
> 
> 4,1313,531166593,-; ? do_error_trap+0x71/0xb0
> 
> 4,1314,531166601,-; ? usercopy_abort+0x6c/0x80
> 
> 4,1315,531166610,-; ? exc_invalid_op+0x52/0x80
> 
> 4,1316,531166622,-; ? usercopy_abort+0x6c/0x80
> 
> 4,1317,531166630,-; ? asm_exc_invalid_op+0x1b/0x20
> 
> 4,1318,531166643,-; ? usercopy_abort+0x6c/0x80
> 
> 4,1319,531166652,-; __check_heap_object+0xe3/0x120
> 
> 4,1320,531166661,-; check_heap_object+0x185/0x1d0
> 
> 4,1321,531166670,-; __check_object_size.part.0+0x72/0x150
> 
> 4,1322,531166679,-; __check_object_size+0x23/0x30
> 
> 4,1323,531166688,-; bsg_transport_sg_io_fn+0x314/0x3b0
> 
> 4,1324,531166699,-; ? __pfx_bsg_transport_sg_io_fn+0x10/0x10
> 
> 4,1325,531166707,-; bsg_sg_io+0x9e/0x120
> 
> 4,1326,531166717,-; bsg_ioctl+0x1f4/0x240
> 
> 4,1327,531166723,-; __x64_sys_ioctl+0x9d/0xe0
> 
> 4,1328,531166734,-; do_syscall_64+0x58/0x90
> 
> 4,1329,531166743,-; ? putname+0x5d/0x80
> 
> 4,1330,531166752,-; ? do_sys_openat2+0xab/0x180
> 
> 4,1331,531166761,-; ? exit_to_user_mode_prepare+0x30/0xb0
> 
> 4,1332,531166771,-; ? syscall_exit_to_user_mode+0x29/0x50
> 
> 4,1333,531166781,-; ? do_syscall_64+0x67/0x90
> 
> 4,1334,531166788,-; ? irqentry_exit_to_user_mode+0x9/0x20
> 
> 4,1335,531166798,-; ? irqentry_exit+0x43/0x50
> 
> 4,1336,531166806,-; ? exc_page_fault+0x94/0x1b0
> 
> 4,1337,531166815,-; entry_SYSCALL_64_after_hwframe+0x72/0xdc
> 
> 4,1338,531166824,-;RIP: 0033:0x45759f
> 
> 4,1339,531166871,-;Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04
> 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00
> 00 00 0f 05 <41> 89 c0 3d 00 f0 ff ff 77 1f 48 8b 44 24 18 64 48 2b
> 04 25 28 00
> 
> 4,1340,531166877,-;RSP: 002b:00007ffe9126eba0 EFLAGS: 00000246
> ORIG_RAX: 0000000000000010
> 
> 

very strange,  I didn't reproduce this issue with the same command, but
I saw the problem job->result was not updated.


it should not be job->reply_len issue, since we initiated the
max_response_len, then:

int len = min(hdr->max_response_len, job->reply_len);


could you check if this works:


diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index 8d4ad0a3f2cf..943382b142ca 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -195,9 +195,9 @@ static int ufs_bsg_request(struct bsg_job *job)
        kfree(buff);
        bsg_reply->result = ret;
        job->reply_len = !rpmb ? sizeof(struct ufs_bsg_reply) :
sizeof(struct ufs_rpmb_reply);
-       /* complete the job here only if no error */
-       if (ret == 0)
-               bsg_job_done(job, ret, bsg_reply-
>reply_payload_rcv_len);
+
+       /* complete the job here */
+       bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
 
        return ret;
 }


Kind regards,
Bean

RE: [PATCH v2] ufs: core: bsg: Fix memory crash in case arpmb command failed

[Arthur Simchaev]

> 
> very strange,  I didn't reproduce this issue with the same command, but I saw
> the problem job->result was not updated.
> 
> 
> it should not be job->reply_len issue, since we initiated the max_response_len,
> then:
> 
> int len = min(hdr->max_response_len, job->reply_len);
> 
> 
> could you check if this works:
> 
> 
> diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c index
> 8d4ad0a3f2cf..943382b142ca 100644
> --- a/drivers/ufs/core/ufs_bsg.c
> +++ b/drivers/ufs/core/ufs_bsg.c
> @@ -195,9 +195,9 @@ static int ufs_bsg_request(struct bsg_job *job)
>         kfree(buff);
>         bsg_reply->result = ret;
>         job->reply_len = !rpmb ? sizeof(struct ufs_bsg_reply) :
> sizeof(struct ufs_rpmb_reply);
> -       /* complete the job here only if no error */
> -       if (ret == 0)
> -               bsg_job_done(job, ret, bsg_reply-
> >reply_payload_rcv_len);
> +
> +       /* complete the job here */
> +       bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
> 
>         return ret;
>  }
> 
> 
> Kind regards,
> Bean

Also, this change fix the crash, but ufs-utils didn't get the error message
With my patch the tool got an error value of -22 (-EINVAL), as expected

Regards
Arthur

Re: [PATCH v2] ufs: core: bsg: Fix memory crash in case arpmb command failed

[Bean Huo]

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

Re: [PATCH v2] ufs: core: bsg: Fix memory crash in case arpmb command failed

[Bart Van Assche]

On 2/18/25 3:15 AM, Arthur Simchaev wrote:
> diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
> index 8d4ad0a3f2cf..a8ed9bc6e4f1 100644
> --- a/drivers/ufs/core/ufs_bsg.c
> +++ b/drivers/ufs/core/ufs_bsg.c
> @@ -194,10 +194,12 @@ static int ufs_bsg_request(struct bsg_job *job)
>   	ufshcd_rpm_put_sync(hba);
>   	kfree(buff);
>   	bsg_reply->result = ret;
> -	job->reply_len = !rpmb ? sizeof(struct ufs_bsg_reply) : sizeof(struct ufs_rpmb_reply);
>   	/* complete the job here only if no error */
> -	if (ret == 0)
> +	if (ret == 0) {
> +		job->reply_len = !rpmb ? sizeof(struct ufs_bsg_reply) :
> +					 sizeof(struct ufs_rpmb_reply);
>   		bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
> +	}

Please make this code easier to read by changing !rpmb into rpmb and by
swapping the two sizeof() expressions.

Thanks,

Bart.

RE: [PATCH v2] ufs: core: bsg: Fix memory crash in case arpmb command failed

[Arthur Simchaev]

> 
> Please make this code easier to read by changing !rpmb into rpmb and by
> swapping the two sizeof() expressions.
> 
> Thanks,
> 
> Bart.

Done