[Qemu-devel] [PATCH] qdev-property: Make bit property parsing stricter

[Qemu-devel] [PATCH] qdev-property: Make bit property parsing stricter

[Jan Kiszka]

By using strncasecmp, we allow for arbitrary characters after the
"on"/"off" string. Fix this by switching to strcasecmp.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/qdev-properties.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 02f0dae..ea3b2df 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -40,9 +40,9 @@ static void qdev_prop_cpy(DeviceState *dev, Property *props, void *src)
 /* Bit */
 static int parse_bit(DeviceState *dev, Property *prop, const char *str)
 {
-    if (!strncasecmp(str, "on", 2))
+    if (!strcasecmp(str, "on"))
         bit_prop_set(dev, prop, true);
-    else if (!strncasecmp(str, "off", 3))
+    else if (!strcasecmp(str, "off"))
         bit_prop_set(dev, prop, false);
     else
         return -EINVAL;
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH] qdev-property: Make bit property parsing stricter

[Andreas Färber]

Am 21.01.2012 14:43, schrieb Jan Kiszka:
> By using strncasecmp, we allow for arbitrary characters after the
> "on"/"off" string. Fix this by switching to strcasecmp.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Reviewed-by: Andreas Färber <afaerber@suse.de>

An alternative might be to increase the char count by one. For a const
char* parameter I see the responsibility for nul-terminating at the
caller though, so this seems right. Did you check all callers?

Andreas

> ---
>  hw/qdev-properties.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 02f0dae..ea3b2df 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -40,9 +40,9 @@ static void qdev_prop_cpy(DeviceState *dev, Property *props, void *src)
>  /* Bit */
>  static int parse_bit(DeviceState *dev, Property *prop, const char *str)
>  {
> -    if (!strncasecmp(str, "on", 2))
> +    if (!strcasecmp(str, "on"))
>          bit_prop_set(dev, prop, true);
> -    else if (!strncasecmp(str, "off", 3))
> +    else if (!strcasecmp(str, "off"))
>          bit_prop_set(dev, prop, false);
>      else
>          return -EINVAL;

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] qdev-property: Make bit property parsing stricter

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 798 bytes --]

On 2012-01-21 17:36, Andreas Färber wrote:
> Am 21.01.2012 14:43, schrieb Jan Kiszka:
>> By using strncasecmp, we allow for arbitrary characters after the
>> "on"/"off" string. Fix this by switching to strcasecmp.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> 
> An alternative might be to increase the char count by one. For a const
> char* parameter I see the responsibility for nul-terminating at the
> caller though, so this seems right. Did you check all callers?

For any normal function that takes a char * string without a maximum
length, the responsibility for terminating is on caller side. Also
because we checks against patterns of different length. So we don't need
to jump through hoops here.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH] qdev-property: Make bit property parsing stricter

[Markus Armbruster]

Jan Kiszka <jan.kiszka@web.de> writes:

> On 2012-01-21 17:36, Andreas Färber wrote:
>> Am 21.01.2012 14:43, schrieb Jan Kiszka:
>>> By using strncasecmp, we allow for arbitrary characters after the
>>> "on"/"off" string. Fix this by switching to strcasecmp.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> 
>> Reviewed-by: Andreas Färber <afaerber@suse.de>
>> 
>> An alternative might be to increase the char count by one. For a const
>> char* parameter I see the responsibility for nul-terminating at the
>> caller though, so this seems right. Did you check all callers?
>
> For any normal function that takes a char * string without a maximum
> length, the responsibility for terminating is on caller side. Also
> because we checks against patterns of different length. So we don't need
> to jump through hoops here.

I agree.

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [Qemu-trivial] [PATCH] qdev-property: Make bit property parsing stricter

[Stefan Hajnoczi]

On Sat, Jan 21, 2012 at 02:43:07PM +0100, Jan Kiszka wrote:
> By using strncasecmp, we allow for arbitrary characters after the
> "on"/"off" string. Fix this by switching to strcasecmp.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/qdev-properties.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan