diff mbox

[PATCHv4] qdev: Validate hex properties

Message ID 1385707735-6994-1-git-send-email-hare@suse.de
State New
Headers show

Commit Message

Hannes Reinecke Nov. 29, 2013, 6:48 a.m. UTC 
strtoul(l) might overflow, in which case it'll return '-1' and set
the appropriate error code. So update the calls to strtoul(l) when
parsing hex properties to avoid silent overflows.
And we should be using an intermediate variable to avoid clobbering
of the passed-in point on error.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 hw/core/qdev-properties.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

Comments

Eric Blake Dec. 2, 2013, 5:52 p.m. UTC | #1 
On 11/28/2013 11:48 PM, Hannes Reinecke wrote:
> strtoul(l) might overflow, in which case it'll return '-1' and set
> the appropriate error code. So update the calls to strtoul(l) when
> parsing hex properties to avoid silent overflows.
> And we should be using an intermediate variable to avoid clobbering
> of the passed-in point on error.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  hw/core/qdev-properties.c | 36 ++++++++++++++++++++++++++++++------
>  1 file changed, 30 insertions(+), 6 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox

Patch

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index dc8ae69..6e4e9f3 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -191,6 +191,7 @@  PropertyInfo qdev_prop_uint8 = {
 
 static int parse_hex8(DeviceState *dev, Property *prop, const char *str)
 {
+    unsigned long val;
     uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
     char *end;
 
@@ -198,11 +199,18 @@  static int parse_hex8(DeviceState *dev, Property *prop, const char *str)
         return -EINVAL;
     }
 
-    *ptr = strtoul(str, &end, 16);
+    errno = 0;
+    val = strtoul(str, &end, 16);
+    if (errno) {
+        return -errno;
+    }
+    if (val > UINT8_MAX) {
+        return -ERANGE;
+    }
     if ((*end != '\0') || (end == str)) {
         return -EINVAL;
     }
-
+    *ptr = val;
     return 0;
 }
 
@@ -322,6 +330,7 @@  PropertyInfo qdev_prop_int32 = {
 
 static int parse_hex32(DeviceState *dev, Property *prop, const char *str)
 {
+    unsigned long val;
     uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
     char *end;
 
@@ -329,11 +338,18 @@  static int parse_hex32(DeviceState *dev, Property *prop, const char *str)
         return -EINVAL;
     }
 
-    *ptr = strtoul(str, &end, 16);
+    errno = 0;
+    val = strtoul(str, &end, 16);
+    if (errno) {
+        return -errno;
+    }
+    if (val > UINT32_MAX) {
+        return -ERANGE;
+    }
     if ((*end != '\0') || (end == str)) {
         return -EINVAL;
     }
-
+    *ptr = val;
     return 0;
 }
 
@@ -389,6 +405,7 @@  PropertyInfo qdev_prop_uint64 = {
 
 static int parse_hex64(DeviceState *dev, Property *prop, const char *str)
 {
+    unsigned long long val;
     uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
     char *end;
 
@@ -396,11 +413,18 @@  static int parse_hex64(DeviceState *dev, Property *prop, const char *str)
         return -EINVAL;
     }
 
-    *ptr = strtoull(str, &end, 16);
+    errno = 0;
+    val = strtoull(str, &end, 16);
+    if (errno) {
+        return -errno;
+    }
+    if (val > UINT64_MAX) {
+        return -ERANGE;
+    }
     if ((*end != '\0') || (end == str)) {
         return -EINVAL;
     }
-
+    *ptr = val;
     return 0;
 }