[PATCH] firmware: qcom: uefisecapp: Fix GetNextVariableName overflow

[PATCH] firmware: qcom: uefisecapp: Fix GetNextVariableName overflow

[Guanglei Zhu]

From: Weibin Liu <liuwb@xiaopeng.com>

qsee_uefi_get_next_variable() checks both variable-sized parameters of
the response before using them:

	if (rsp_data->name_offset + rsp_data->name_size > rsp_data->length)
		return EFI_DEVICE_ERROR;

	if (rsp_data->guid_offset + rsp_data->guid_size > rsp_data->length)
		return EFI_DEVICE_ERROR;

Every operand is a u32 taken from the response buffer, so both sums are
computed modulo 2^32.  A response with name_offset = 0xFFFFF000 and
name_size = 0x2000 wraps around to zero and passes the check; the same
holds for guid_offset and guid_size.  The name then reaches
ucs2_strscpy() as a source pointer far outside the response buffer,
which reads until it finds a NUL or exhausts the count, and the GUID is
copied from such a pointer as well.

Because wrapping around requires the offset itself to be huge, the read
lands in unmapped memory and faults, taking the kernel down; if the
address happens to be mapped, kernel memory ends up in the caller's
name or GUID buffer.

Rewrite both checks without an addition: reject the offset first when
it is larger than length, then compare the size against the room that
is left, which cannot overflow.

Fixes: 759e7a2b62eb ("firmware: Add support for Qualcomm UEFI Secure Application")
Cc: stable@vger.kernel.org
Signed-off-by: Weibin Liu <liuwb@xiaopeng.com>
---

Verified the wraparound with a userspace replica of the response
parsing: name_offset = 0xfffff000 with name_size = 0x1100 passes
the old check (the sum wraps to 0x100 == length) and faults in
ucs2_strscpy(); with the offset landing on mapped memory instead,
the content of a page roughly 3 GiB past the response buffer ends
up in the caller's name buffer.

 drivers/firmware/qcom/qcom_qseecom_uefisecapp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c b/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
index 0dda09ad156a..2c8f6dfc23b6 100644
--- a/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
+++ b/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
@@ -605,10 +605,12 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
 	if (rsp_data->length > rsp_size)
 		return EFI_DEVICE_ERROR;
 
-	if (rsp_data->name_offset + rsp_data->name_size > rsp_data->length)
+	if (rsp_data->name_offset > rsp_data->length ||
+	    rsp_data->name_size > rsp_data->length - rsp_data->name_offset)
 		return EFI_DEVICE_ERROR;
 
-	if (rsp_data->guid_offset + rsp_data->guid_size > rsp_data->length)
+	if (rsp_data->guid_offset > rsp_data->length ||
+	    rsp_data->guid_size > rsp_data->length - rsp_data->guid_offset)
 		return EFI_DEVICE_ERROR;
 
 	if (rsp_data->name_size > *name_size) {
-- 
2.50.1