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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8be8c9c45d627e40e4ce3dc87c1ac83f32717e2b.camel@gmail.com>
Date: Tue, 18 Feb 2025 13:31:23 +0100
From: Bean Huo <huobean@...il.com>
To: Arthur Simchaev <arthur.simchaev@...disk.com>, martin.petersen@...cle.com
Cc: avri.altman@...disk.com, Avi.Shchislowski@...disk.com,
 beanhuo@...ron.com,  linux-scsi@...r.kernel.org,
 linux-kernel@...r.kernel.org, bvanassche@....org,  stable@...r.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@...r.kernel.org
> Signed-off-by: Arthur Simchaev <arthur.simchaev@...disk.com>
> 
> ---
> Changes in v2:
>   - Add Fixes tag
>   - Elaborate commit log
> 
> Signed-off-by: Arthur Simchaev <arthur.simchaev@...disk.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