linux-kernel - [PATCH 4.9 19/61] scsi: ufs: fix failure to read the string descriptor

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]

Date:   Tue,  5 Jun 2018 19:01:40 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org,
        Venkat Gopalakrishnan <venkatg@...eaurora.org>,
        Subhash Jadavani <subhashj@...eaurora.org>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Li Wei <liwei213@...wei.com>
Subject: [PATCH 4.9 19/61] scsi: ufs: fix failure to read the string descriptor

4.9-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Subhash Jadavani <subhashj@...eaurora.org>

commit bde44bb665d049468b6a1a2fa7d666434de4f83f upstream.

While reading variable size descriptors (like string descriptor), some UFS
devices may report the "LENGTH" (field in "Transaction Specific fields" of
Query Response UPIU) same as what was requested in Query Request UPIU
instead of reporting the actual size of the variable size descriptor.
Although it's safe to ignore the "LENGTH" field for variable size
descriptors as we can always derive the length of the descriptor from
the descriptor header fields. Hence this change impose the length match
check only for fixed size descriptors (for which we always request the
correct size as part of Query Request UPIU).

Reviewed-by: Venkat Gopalakrishnan <venkatg@...eaurora.org>
Signed-off-by: Subhash Jadavani <subhashj@...eaurora.org>
Signed-off-by: Martin K. Petersen <martin.petersen@...cle.com>
[Wei Li: Slight tweaks to get the cherry-pick to apply,resolved collisions.]
Signed-off-by: Li Wei <liwei213@...wei.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>

---
 drivers/scsi/ufs/ufshcd.c |   39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2086,18 +2086,41 @@ static int ufshcd_read_desc_param(struct
 					desc_id, desc_index, 0, desc_buf,
 					&buff_len);
 
-	if (ret || (buff_len < ufs_query_desc_max_size[desc_id]) ||
-	    (desc_buf[QUERY_DESC_LENGTH_OFFSET] !=
-	     ufs_query_desc_max_size[desc_id])
-	    || (desc_buf[QUERY_DESC_DESC_TYPE_OFFSET] != desc_id)) {
-		dev_err(hba->dev, "%s: Failed reading descriptor. desc_id %d param_offset %d buff_len %d ret %d",
-			__func__, desc_id, param_offset, buff_len, ret);
-		if (!ret)
-			ret = -EINVAL;
+	if (ret) {
+		dev_err(hba->dev, "%s: Failed reading descriptor. desc_id %d, desc_index %d, param_offset %d, ret %d",
+			__func__, desc_id, desc_index, param_offset, ret);
 
 		goto out;
 	}
 
+	/* Sanity check */
+	if (desc_buf[QUERY_DESC_DESC_TYPE_OFFSET] != desc_id) {
+		dev_err(hba->dev, "%s: invalid desc_id %d in descriptor header",
+			__func__, desc_buf[QUERY_DESC_DESC_TYPE_OFFSET]);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/*
+	 * While reading variable size descriptors (like string descriptor),
+	 * some UFS devices may report the "LENGTH" (field in "Transaction
+	 * Specific fields" of Query Response UPIU) same as what was requested
+	 * in Query Request UPIU instead of reporting the actual size of the
+	 * variable size descriptor.
+	 * Although it's safe to ignore the "LENGTH" field for variable size
+	 * descriptors as we can always derive the length of the descriptor from
+	 * the descriptor header fields. Hence this change impose the length
+	 * match check only for fixed size descriptors (for which we always
+	 * request the correct size as part of Query Request UPIU).
+	 */
+	if ((desc_id != QUERY_DESC_IDN_STRING) &&
+	    (buff_len != desc_buf[QUERY_DESC_LENGTH_OFFSET])) {
+		dev_err(hba->dev, "%s: desc_buf length mismatch: buff_len %d, buff_len(desc_header) %d",
+			__func__, buff_len, desc_buf[QUERY_DESC_LENGTH_OFFSET]);
+		ret = -EINVAL;
+		goto out;
+	}
+
 	if (is_kmalloc)
 		memcpy(param_read_buf, &desc_buf[param_offset], param_size);
 out: