The QMI TLV value for strings in a lot of qmi element info structures
account for null terminated strings with MAX_LEN + 1. If a string is
actually MAX_LEN + 1 length, this will cause an out of bounds access
when the NULL character is appended in decoding.
Fixes: 9b8a11e82615 ("soc: qcom: Introduce QMI encoder/decoder")
Cc: [email protected]
Signed-off-by: Chris Lew <[email protected]>
Signed-off-by: Praveenkumar I <[email protected]>
---
[v2]:
Added Fixes and Cc: stable
drivers/soc/qcom/qmi_encdec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
index b7158e3c3a0b..5c7161b18b72 100644
--- a/drivers/soc/qcom/qmi_encdec.c
+++ b/drivers/soc/qcom/qmi_encdec.c
@@ -534,8 +534,8 @@ static int qmi_decode_string_elem(const struct qmi_elem_info *ei_array,
decoded_bytes += rc;
}
- if (string_len > temp_ei->elem_len) {
- pr_err("%s: String len %d > Max Len %d\n",
+ if (string_len >= temp_ei->elem_len) {
+ pr_err("%s: String len %d >= Max Len %d\n",
__func__, string_len, temp_ei->elem_len);
return -ETOOSMALL;
} else if (string_len > tlv_len) {
--
2.34.1
On Mon, Jul 31, 2023 at 03:33:11PM +0530, Praveenkumar I wrote:
> The QMI TLV value for strings in a lot of qmi element info structures
> account for null terminated strings with MAX_LEN + 1. If a string is
> actually MAX_LEN + 1 length, this will cause an out of bounds access
> when the NULL character is appended in decoding.
>
> Fixes: 9b8a11e82615 ("soc: qcom: Introduce QMI encoder/decoder")
> Cc: [email protected]
> Signed-off-by: Chris Lew <[email protected]>
> Signed-off-by: Praveenkumar I <[email protected]>
The signed-off-by list says that Chris certified the patch's origin
first, then you took it, certified the origin and submitted it to the
mailing list.
This matches reality, but you lost Chris' authorship in the process,
please add that back.
Thanks,
Bjorn
> ---
> [v2]:
> Added Fixes and Cc: stable
>
> drivers/soc/qcom/qmi_encdec.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
> index b7158e3c3a0b..5c7161b18b72 100644
> --- a/drivers/soc/qcom/qmi_encdec.c
> +++ b/drivers/soc/qcom/qmi_encdec.c
> @@ -534,8 +534,8 @@ static int qmi_decode_string_elem(const struct qmi_elem_info *ei_array,
> decoded_bytes += rc;
> }
>
> - if (string_len > temp_ei->elem_len) {
> - pr_err("%s: String len %d > Max Len %d\n",
> + if (string_len >= temp_ei->elem_len) {
> + pr_err("%s: String len %d >= Max Len %d\n",
> __func__, string_len, temp_ei->elem_len);
> return -ETOOSMALL;
> } else if (string_len > tlv_len) {
> --
> 2.34.1
>
On 8/1/2023 4:54 AM, Bjorn Andersson wrote:
> On Mon, Jul 31, 2023 at 03:33:11PM +0530, Praveenkumar I wrote:
>> The QMI TLV value for strings in a lot of qmi element info structures
>> account for null terminated strings with MAX_LEN + 1. If a string is
>> actually MAX_LEN + 1 length, this will cause an out of bounds access
>> when the NULL character is appended in decoding.
>>
>> Fixes: 9b8a11e82615 ("soc: qcom: Introduce QMI encoder/decoder")
>> Cc: [email protected]
>> Signed-off-by: Chris Lew <[email protected]>
>> Signed-off-by: Praveenkumar I <[email protected]>
> The signed-off-by list says that Chris certified the patch's origin
> first, then you took it, certified the origin and submitted it to the
> mailing list.
>
> This matches reality, but you lost Chris' authorship in the process,
> please add that back.
Yes, you are right. Will add that, and post it.
- Praveenkumar
> Thanks,
> Bjorn
>
>> ---
>> [v2]:
>> Added Fixes and Cc: stable
>>
>> drivers/soc/qcom/qmi_encdec.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
>> index b7158e3c3a0b..5c7161b18b72 100644
>> --- a/drivers/soc/qcom/qmi_encdec.c
>> +++ b/drivers/soc/qcom/qmi_encdec.c
>> @@ -534,8 +534,8 @@ static int qmi_decode_string_elem(const struct qmi_elem_info *ei_array,
>> decoded_bytes += rc;
>> }
>>
>> - if (string_len > temp_ei->elem_len) {
>> - pr_err("%s: String len %d > Max Len %d\n",
>> + if (string_len >= temp_ei->elem_len) {
>> + pr_err("%s: String len %d >= Max Len %d\n",
>> __func__, string_len, temp_ei->elem_len);
>> return -ETOOSMALL;
>> } else if (string_len > tlv_len) {
>> --
>> 2.34.1
>>