* [Qemu-devel] [PATCH for-3.1] replay: Exit on errors reading from replay log
@ 2018-11-06 15:33 Peter Maydell
2018-11-06 22:19 ` Paolo Bonzini
2018-11-07 7:50 ` Pavel Dovgalyuk
0 siblings, 2 replies; 4+ messages in thread
From: Peter Maydell @ 2018-11-06 15:33 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, Paolo Bonzini, Pavel Dovgalyuk
Currently replay_get_byte() does not check for an error
from getc(). Coverity points out (CID 1390622) that this
could result in unexpected behaviour (such as looping
forever, if we use the replay_get_dword() return value
for a loop count). We don't expect reads from the replay
log to fail, and if they do there is no way we can
continue. So make them fatal errors.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Disclaimer: checked only with "make check".
replay/replay-internal.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/replay/replay-internal.c b/replay/replay-internal.c
index 1cea1d4dc91..8f87e9b957e 100644
--- a/replay/replay-internal.c
+++ b/replay/replay-internal.c
@@ -35,6 +35,12 @@ static void replay_write_error(void)
}
}
+static void replay_read_error(void)
+{
+ error_report("error reading the replay data");
+ exit(1);
+}
+
void replay_put_byte(uint8_t byte)
{
if (replay_file) {
@@ -83,7 +89,11 @@ uint8_t replay_get_byte(void)
{
uint8_t byte = 0;
if (replay_file) {
- byte = getc(replay_file);
+ int r = getc(replay_file);
+ if (r == EOF) {
+ replay_read_error();
+ }
+ byte = r;
}
return byte;
}
@@ -126,7 +136,7 @@ void replay_get_array(uint8_t *buf, size_t *size)
if (replay_file) {
*size = replay_get_dword();
if (fread(buf, 1, *size, replay_file) != *size) {
- error_report("replay read error");
+ replay_read_error();
}
}
}
@@ -137,7 +147,7 @@ void replay_get_array_alloc(uint8_t **buf, size_t *size)
*size = replay_get_dword();
*buf = g_malloc(*size);
if (fread(*buf, 1, *size, replay_file) != *size) {
- error_report("replay read error");
+ replay_read_error();
}
}
}
--
2.19.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-3.1] replay: Exit on errors reading from replay log
2018-11-06 15:33 [Qemu-devel] [PATCH for-3.1] replay: Exit on errors reading from replay log Peter Maydell
@ 2018-11-06 22:19 ` Paolo Bonzini
2018-11-08 14:42 ` Peter Maydell
2018-11-07 7:50 ` Pavel Dovgalyuk
1 sibling, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2018-11-06 22:19 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, Pavel Dovgalyuk
On 06/11/2018 16:33, Peter Maydell wrote:
> Currently replay_get_byte() does not check for an error
> from getc(). Coverity points out (CID 1390622) that this
> could result in unexpected behaviour (such as looping
> forever, if we use the replay_get_dword() return value
> for a loop count). We don't expect reads from the replay
> log to fail, and if they do there is no way we can
> continue. So make them fatal errors.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Disclaimer: checked only with "make check".
>
> replay/replay-internal.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
> index 1cea1d4dc91..8f87e9b957e 100644
> --- a/replay/replay-internal.c
> +++ b/replay/replay-internal.c
> @@ -35,6 +35,12 @@ static void replay_write_error(void)
> }
> }
>
> +static void replay_read_error(void)
> +{
> + error_report("error reading the replay data");
> + exit(1);
> +}
> +
> void replay_put_byte(uint8_t byte)
> {
> if (replay_file) {
> @@ -83,7 +89,11 @@ uint8_t replay_get_byte(void)
> {
> uint8_t byte = 0;
> if (replay_file) {
> - byte = getc(replay_file);
> + int r = getc(replay_file);
> + if (r == EOF) {
> + replay_read_error();
> + }
> + byte = r;
> }
> return byte;
> }
> @@ -126,7 +136,7 @@ void replay_get_array(uint8_t *buf, size_t *size)
> if (replay_file) {
> *size = replay_get_dword();
> if (fread(buf, 1, *size, replay_file) != *size) {
> - error_report("replay read error");
> + replay_read_error();
> }
> }
> }
> @@ -137,7 +147,7 @@ void replay_get_array_alloc(uint8_t **buf, size_t *size)
> *size = replay_get_dword();
> *buf = g_malloc(*size);
> if (fread(*buf, 1, *size, replay_file) != *size) {
> - error_report("replay read error");
> + replay_read_error();
> }
> }
> }
>
Makes sense, can you apply it directly to qemu.git as soon as Pavel
reviews it (or in some time if he doesn't)?
Thanks,
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-3.1] replay: Exit on errors reading from replay log
2018-11-06 15:33 [Qemu-devel] [PATCH for-3.1] replay: Exit on errors reading from replay log Peter Maydell
2018-11-06 22:19 ` Paolo Bonzini
@ 2018-11-07 7:50 ` Pavel Dovgalyuk
1 sibling, 0 replies; 4+ messages in thread
From: Pavel Dovgalyuk @ 2018-11-07 7:50 UTC (permalink / raw)
To: 'Peter Maydell', qemu-devel
Cc: patches, 'Paolo Bonzini', 'Pavel Dovgalyuk'
> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> Currently replay_get_byte() does not check for an error
> from getc(). Coverity points out (CID 1390622) that this
> could result in unexpected behaviour (such as looping
> forever, if we use the replay_get_dword() return value
> for a loop count). We don't expect reads from the replay
> log to fail, and if they do there is no way we can
> continue. So make them fatal errors.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
> Disclaimer: checked only with "make check".
>
> replay/replay-internal.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
> index 1cea1d4dc91..8f87e9b957e 100644
> --- a/replay/replay-internal.c
> +++ b/replay/replay-internal.c
> @@ -35,6 +35,12 @@ static void replay_write_error(void)
> }
> }
>
> +static void replay_read_error(void)
> +{
> + error_report("error reading the replay data");
> + exit(1);
> +}
> +
> void replay_put_byte(uint8_t byte)
> {
> if (replay_file) {
> @@ -83,7 +89,11 @@ uint8_t replay_get_byte(void)
> {
> uint8_t byte = 0;
> if (replay_file) {
> - byte = getc(replay_file);
> + int r = getc(replay_file);
> + if (r == EOF) {
> + replay_read_error();
> + }
> + byte = r;
> }
> return byte;
> }
> @@ -126,7 +136,7 @@ void replay_get_array(uint8_t *buf, size_t *size)
> if (replay_file) {
> *size = replay_get_dword();
> if (fread(buf, 1, *size, replay_file) != *size) {
> - error_report("replay read error");
> + replay_read_error();
> }
> }
> }
> @@ -137,7 +147,7 @@ void replay_get_array_alloc(uint8_t **buf, size_t *size)
> *size = replay_get_dword();
> *buf = g_malloc(*size);
> if (fread(*buf, 1, *size, replay_file) != *size) {
> - error_report("replay read error");
> + replay_read_error();
> }
> }
> }
> --
> 2.19.1
Pavel Dovgalyuk
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-3.1] replay: Exit on errors reading from replay log
2018-11-06 22:19 ` Paolo Bonzini
@ 2018-11-08 14:42 ` Peter Maydell
0 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2018-11-08 14:42 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: QEMU Developers, patches, Pavel Dovgalyuk
On 6 November 2018 at 22:19, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 06/11/2018 16:33, Peter Maydell wrote:
>> Currently replay_get_byte() does not check for an error
>> from getc(). Coverity points out (CID 1390622) that this
>> could result in unexpected behaviour (such as looping
>> forever, if we use the replay_get_dword() return value
>> for a loop count). We don't expect reads from the replay
>> log to fail, and if they do there is no way we can
>> continue. So make them fatal errors.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
> Makes sense, can you apply it directly to qemu.git as soon as Pavel
> reviews it (or in some time if he doesn't)?
Applied to master, thanks.
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-11-08 14:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-06 15:33 [Qemu-devel] [PATCH for-3.1] replay: Exit on errors reading from replay log Peter Maydell
2018-11-06 22:19 ` Paolo Bonzini
2018-11-08 14:42 ` Peter Maydell
2018-11-07 7:50 ` Pavel Dovgalyuk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).