[PATCH] replay: Improve assert in replay_char_read_all_load()

Peter Maydell posted 1 patch 2 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251124173407.50124-1-peter.maydell@linaro.org
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>
replay/replay-char.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] replay: Improve assert in replay_char_read_all_load()
Posted by Peter Maydell 2 months, 2 weeks ago
In replay_char_read_all_load() we get a buffer and size from the
replay log.  We know the size has to fit an int because of how we
write the log.  However the way we assert this is wrong: we cast the
size_t from replay_get_array() to an int and then check that it is
non-negative.  This misses cases where an over-large size is
truncated into a positive value by the cast.

Replace the assertion with checking that the size is in-range
before doing the cast.

Coverity complained about the possible overflow: CID 1643440.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 replay/replay-char.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/replay/replay-char.c b/replay/replay-char.c
index 81dc416e988..4e58dd154af 100644
--- a/replay/replay-char.c
+++ b/replay/replay-char.c
@@ -126,8 +126,8 @@ int replay_char_read_all_load(uint8_t *buf)
         int res;
         replay_get_array(buf, &size);
         replay_finish_event();
+        assert(size <= INT_MAX);
         res = (int)size;
-        assert(res >= 0);
         return res;
     } else if (replay_next_event_is(EVENT_CHAR_READ_ALL_ERROR)) {
         int res = replay_get_dword();
-- 
2.43.0
Re: [PATCH] replay: Improve assert in replay_char_read_all_load()
Posted by Philippe Mathieu-Daudé 2 months, 2 weeks ago
On 24/11/25 18:34, Peter Maydell wrote:
> In replay_char_read_all_load() we get a buffer and size from the
> replay log.  We know the size has to fit an int because of how we
> write the log.  However the way we assert this is wrong: we cast the
> size_t from replay_get_array() to an int and then check that it is
> non-negative.  This misses cases where an over-large size is
> truncated into a positive value by the cast.
> 
> Replace the assertion with checking that the size is in-range
> before doing the cast.
> 
> Coverity complained about the possible overflow: CID 1643440.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   replay/replay-char.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Queued, thanks.
Re: [PATCH] replay: Improve assert in replay_char_read_all_load()
Posted by Philippe Mathieu-Daudé 2 months, 2 weeks ago
On 24/11/25 18:34, Peter Maydell wrote:
> In replay_char_read_all_load() we get a buffer and size from the
> replay log.  We know the size has to fit an int because of how we
> write the log.  However the way we assert this is wrong: we cast the
> size_t from replay_get_array() to an int and then check that it is
> non-negative.  This misses cases where an over-large size is
> truncated into a positive value by the cast.
> 
> Replace the assertion with checking that the size is in-range
> before doing the cast.
> 
> Coverity complained about the possible overflow: CID 1643440.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   replay/replay-char.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>