| Message ID | 20250321092623.2097234-3-jamin_lin@aspeedtech.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix incorrect hash results on AST2700 | expand |
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 ")
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.
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;
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(+)