| Message ID | 20250218111527.246506-1-arthur.simchaev@sandisk.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] ufs: core: bsg: Fix memory crash in case arpmb command failed | expand |
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
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
Reviewed-by: Bean Huo <beanhuo@micron.com>
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; }
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(-)