Thread (66 messages) 66 messages, 9 authors, 2021-09-02

RE: [PATCH v3 01/18] scsi: ufs: Fix memory corruption by ufshcd_read_desc_param()

From: Avri Altman <Avri.Altman@wdc.com>
Date: 2021-07-25 12:40:42

If param_offset > buff_len then the memcpy() statement in
ufshcd_read_desc_param() corrupts memory since it copies
256 + buff_len - param_offset bytes into a buffer with size buff_len.
Since param_offset < 256 this results in writing past the bound of the output
buffer.

Fixes: cbe193f6f093 ("scsi: ufs: Fix potential NULL pointer access during
memcpy")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>

Your fix is fine IMO.
However, the root cause of this weird bug is that rpmb has its own unit descriptor,
But ufshcd_map_desc_id_to_length doesn't accept index as argument, and returned 0x2d instead of 0x23, as it should.

Thanks,
Avri
quoted hunk ↗ jump to hunk
---
 drivers/scsi/ufs/ufshcd.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index
064a44e628d6..6c251afe65f9 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3418,9 +3418,11 @@ int ufshcd_read_desc_param(struct ufs_hba
*hba,

        if (is_kmalloc) {
                /* Make sure we don't copy more data than available */
-               if (param_offset + param_size > buff_len)
-                       param_size = buff_len - param_offset;
-               memcpy(param_read_buf, &desc_buf[param_offset], param_size);
+               if (param_offset >= buff_len)
+                       ret = -EINVAL;
+               else
+                       memcpy(param_read_buf, &desc_buf[param_offset],
+                              min_t(u32, param_size, buff_len -
+ param_offset));
        }
 out:
        if (is_kmalloc)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help