diff mbox

qdev-property: Make bit property parsing stricter

Message ID 4F1AC0EB.3050304@web.de
State New
Headers show

Commit Message

Jan Kiszka Jan. 21, 2012, 1:43 p.m. UTC
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(-)

Comments

Andreas Färber Jan. 21, 2012, 4:36 p.m. UTC | #1
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;
Jan Kiszka Jan. 21, 2012, 5:07 p.m. UTC | #2
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
Markus Armbruster Jan. 24, 2012, 12:05 p.m. UTC | #3
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>
Stefan Hajnoczi Jan. 27, 2012, 5:50 a.m. UTC | #4
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
diff mbox

Patch

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;