* [Qemu-devel] [PATCH for-2.7] qtest.c: Allow zero size in memset qtest commands
@ 2016-08-05 10:43 Peter Maydell
2016-09-06 12:48 ` Peter Maydell
2016-09-08 14:37 ` Eric Blake
0 siblings, 2 replies; 4+ messages in thread
From: Peter Maydell @ 2016-08-05 10:43 UTC (permalink / raw)
To: qemu-devel; +Cc: patches, John Snow, Eric Blake, Markus Armbruster
Some tests use the qtest protocol "memset" command with a zero
size, expecting it to do nothing. However in the current code this
will result in calling memset() with a NULL pointer, which is
undefined behaviour. Detect and specially handle zero sizes to
avoid this.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Looking at the code for the other commands that take a size
('read', 'write', 'b64read' and 'b64write' they all assume a
non-zero size. I've left those alone though, somebody else can
make them do nothing on zero size if they feel it's important.)
qtest.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/qtest.c b/qtest.c
index da4826c..ce4c6db 100644
--- a/qtest.c
+++ b/qtest.c
@@ -133,6 +133,7 @@ static bool qtest_opened;
* < OK
*
* ADDR, SIZE, VALUE are all integers parsed with strtoul() with a base of 0.
+ * For 'memset' a zero size is permitted and does nothing.
*
* DATA is an arbitrarily long hex number prefixed with '0x'. If it's smaller
* than the expected size, the value will be zero filled at the end of the data
@@ -493,10 +494,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
len = strtoull(words[2], NULL, 0);
pattern = strtoull(words[3], NULL, 0);
- data = g_malloc(len);
- memset(data, pattern, len);
- cpu_physical_memory_write(addr, data, len);
- g_free(data);
+ if (len) {
+ data = g_malloc(len);
+ memset(data, pattern, len);
+ cpu_physical_memory_write(addr, data, len);
+ g_free(data);
+ }
qtest_send_prefix(chr);
qtest_send(chr, "OK\n");
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.7] qtest.c: Allow zero size in memset qtest commands
2016-08-05 10:43 [Qemu-devel] [PATCH for-2.7] qtest.c: Allow zero size in memset qtest commands Peter Maydell
@ 2016-09-06 12:48 ` Peter Maydell
2016-09-08 14:37 ` Eric Blake
1 sibling, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2016-09-06 12:48 UTC (permalink / raw)
To: QEMU Developers; +Cc: John Snow, Markus Armbruster, Patch Tracking
Ping?
(Now that 2.7 is out it would be nice to get rid of the clang
warnings cluttering up my build logs :-))
thanks
-- PMM
On 5 August 2016 at 11:43, Peter Maydell <peter.maydell@linaro.org> wrote:
> Some tests use the qtest protocol "memset" command with a zero
> size, expecting it to do nothing. However in the current code this
> will result in calling memset() with a NULL pointer, which is
> undefined behaviour. Detect and specially handle zero sizes to
> avoid this.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Looking at the code for the other commands that take a size
> ('read', 'write', 'b64read' and 'b64write' they all assume a
> non-zero size. I've left those alone though, somebody else can
> make them do nothing on zero size if they feel it's important.)
>
> qtest.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/qtest.c b/qtest.c
> index da4826c..ce4c6db 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -133,6 +133,7 @@ static bool qtest_opened;
> * < OK
> *
> * ADDR, SIZE, VALUE are all integers parsed with strtoul() with a base of 0.
> + * For 'memset' a zero size is permitted and does nothing.
> *
> * DATA is an arbitrarily long hex number prefixed with '0x'. If it's smaller
> * than the expected size, the value will be zero filled at the end of the data
> @@ -493,10 +494,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
> len = strtoull(words[2], NULL, 0);
> pattern = strtoull(words[3], NULL, 0);
>
> - data = g_malloc(len);
> - memset(data, pattern, len);
> - cpu_physical_memory_write(addr, data, len);
> - g_free(data);
> + if (len) {
> + data = g_malloc(len);
> + memset(data, pattern, len);
> + cpu_physical_memory_write(addr, data, len);
> + g_free(data);
> + }
>
> qtest_send_prefix(chr);
> qtest_send(chr, "OK\n");
> --
> 2.7.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.7] qtest.c: Allow zero size in memset qtest commands
2016-08-05 10:43 [Qemu-devel] [PATCH for-2.7] qtest.c: Allow zero size in memset qtest commands Peter Maydell
2016-09-06 12:48 ` Peter Maydell
@ 2016-09-08 14:37 ` Eric Blake
2016-09-09 11:49 ` Peter Maydell
1 sibling, 1 reply; 4+ messages in thread
From: Eric Blake @ 2016-09-08 14:37 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches, John Snow, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 899 bytes --]
On 08/05/2016 05:43 AM, Peter Maydell wrote:
> Some tests use the qtest protocol "memset" command with a zero
> size, expecting it to do nothing. However in the current code this
> will result in calling memset() with a NULL pointer, which is
> undefined behaviour. Detect and specially handle zero sizes to
> avoid this.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Looking at the code for the other commands that take a size
> ('read', 'write', 'b64read' and 'b64write' they all assume a
> non-zero size. I've left those alone though, somebody else can
> make them do nothing on zero size if they feel it's important.)
I obviously missed reviewing this in time for 2.7, but looks reasonable
to me.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.7] qtest.c: Allow zero size in memset qtest commands
2016-09-08 14:37 ` Eric Blake
@ 2016-09-09 11:49 ` Peter Maydell
0 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2016-09-09 11:49 UTC (permalink / raw)
To: Eric Blake; +Cc: QEMU Developers, Patch Tracking, John Snow, Markus Armbruster
On 8 September 2016 at 15:37, Eric Blake <eblake@redhat.com> wrote:
> On 08/05/2016 05:43 AM, Peter Maydell wrote:
>> Some tests use the qtest protocol "memset" command with a zero
>> size, expecting it to do nothing. However in the current code this
>> will result in calling memset() with a NULL pointer, which is
>> undefined behaviour. Detect and specially handle zero sizes to
>> avoid this.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> Looking at the code for the other commands that take a size
>> ('read', 'write', 'b64read' and 'b64write' they all assume a
>> non-zero size. I've left those alone though, somebody else can
>> make them do nothing on zero size if they feel it's important.)
>
> I obviously missed reviewing this in time for 2.7, but looks reasonable
> to me.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Applied to master, thanks.
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-09 11:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-05 10:43 [Qemu-devel] [PATCH for-2.7] qtest.c: Allow zero size in memset qtest commands Peter Maydell
2016-09-06 12:48 ` Peter Maydell
2016-09-08 14:37 ` Eric Blake
2016-09-09 11:49 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.