Fix incorrect hash results on AST2700

[PATCH v1 02/22] hw/misc/aspeed_hace: Fix buffer overflow in has_padding function

Posted by Jamin Lin via 1 week, 3 days ago

The maximum padding size is either 64 or 128 bytes and should always be smaller
than "req_len". If "padding_size" exceeds "req_len", then
"req_len - padding_size" underflows due to "uint32_t" data type, leading to a
large incorrect value (e.g., `0xFFXXXXXX`). This causes an out-of-bounds memory
access, potentially leading to a buffer overflow.

Added a check to ensure "padding_size" does not exceed "req_len" before
computing "pad_offset". This prevents "req_len - padding_size" from underflowing
and avoids accessing invalid memory.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 8e7e8113a5..d8b5f048bb 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -128,6 +128,11 @@ static bool has_padding(AspeedHACEState *s, struct iovec *iov,
     if (*total_msg_len <= s->total_req_len) {
         uint32_t padding_size = s->total_req_len - *total_msg_len;
         uint8_t *padding = iov->iov_base;
+
+        if (padding_size > req_len) {
+            return false;
+        }
+
         *pad_offset = req_len - padding_size;
         if (padding[*pad_offset] == 0x80) {
             return true;
-- 
2.43.0

RE: [PATCH v1 02/22] hw/misc/aspeed_hace: Fix buffer overflow in has_padding function

Posted by Jamin Lin 1 week, 3 days ago

Hi Cedric,

> Subject: [PATCH v1 02/22] hw/misc/aspeed_hace: Fix buffer overflow in
> has_padding function
> 
> The maximum padding size is either 64 or 128 bytes and should always be
> smaller than "req_len". If "padding_size" exceeds "req_len", then "req_len -
> padding_size" underflows due to "uint32_t" data type, leading to a large
> incorrect value (e.g., `0xFFXXXXXX`). This causes an out-of-bounds memory
> access, potentially leading to a buffer overflow.
> 
> Added a check to ensure "padding_size" does not exceed "req_len" before
> computing "pad_offset". This prevents "req_len - padding_size" from
> underflowing and avoids accessing invalid memory.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>  hw/misc/aspeed_hace.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c index
> 8e7e8113a5..d8b5f048bb 100644
> --- a/hw/misc/aspeed_hace.c
> +++ b/hw/misc/aspeed_hace.c
> @@ -128,6 +128,11 @@ static bool has_padding(AspeedHACEState *s, struct
> iovec *iov,
>      if (*total_msg_len <= s->total_req_len) {
>          uint32_t padding_size = s->total_req_len - *total_msg_len;
>          uint8_t *padding = iov->iov_base;
> +
> +        if (padding_size > req_len) {
> +            return false;
> +        }
> +
>          *pad_offset = req_len - padding_size;
>          if (padding[*pad_offset] == 0x80) {
>              return true;
> --
> 2.43.0

Fixes: 5cd7d8564a8b563da724b9e6264c967f0a091afa ("aspeed/hace: Support AST2600 HACE ")

Re: [PATCH v1 02/22] hw/misc/aspeed_hace: Fix buffer overflow in has_padding function

Posted by Cédric Le Goater 1 week, 2 days ago

On 3/21/25 10:47, Jamin Lin wrote:
> Hi Cedric,
> 
>> Subject: [PATCH v1 02/22] hw/misc/aspeed_hace: Fix buffer overflow in
>> has_padding function
>>
>> The maximum padding size is either 64 or 128 bytes and should always be
>> smaller than "req_len". If "padding_size" exceeds "req_len", then "req_len -
>> padding_size" underflows due to "uint32_t" data type, leading to a large
>> incorrect value (e.g., `0xFFXXXXXX`). This causes an out-of-bounds memory
>> access, potentially leading to a buffer overflow.
>>
>> Added a check to ensure "padding_size" does not exceed "req_len" before
>> computing "pad_offset". This prevents "req_len - padding_size" from
>> underflowing and avoids accessing invalid memory.
>>
>> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>> ---
>>   hw/misc/aspeed_hace.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c index
>> 8e7e8113a5..d8b5f048bb 100644
>> --- a/hw/misc/aspeed_hace.c
>> +++ b/hw/misc/aspeed_hace.c
>> @@ -128,6 +128,11 @@ static bool has_padding(AspeedHACEState *s, struct
>> iovec *iov,
>>       if (*total_msg_len <= s->total_req_len) {
>>           uint32_t padding_size = s->total_req_len - *total_msg_len;
>>           uint8_t *padding = iov->iov_base;
>> +
>> +        if (padding_size > req_len) {
>> +            return false;
>> +        }
>> +
>>           *pad_offset = req_len - padding_size;
>>           if (padding[*pad_offset] == 0x80) {
>>               return true;
>> --
>> 2.43.0
> 
> Fixes: 5cd7d8564a8b563da724b9e6264c967f0a091afa ("aspeed/hace: Support AST2600 HACE ")



Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.