[PATCH 0/4] Little/big endian and memory fixes.
This series addresses a number of problems encountered testing libvirt-cim on s390. The first three patches deal with endian-unsafe memory access, which of course only surfaces on big endian systems. The last patch addresses a potential heap corruption/double free issue also hit during testing. Thilo Boehm (2): FilterEntry: Fix endianness issues VSSM: Fix endianness issue in domain properties Viktor Mihajlovski (2): VSDC: Fix endianess issues libxkutil: clean entire device structure to avoid memory corruption libxkutil/device_parsing.c | 4 +-- libxkutil/device_parsing.h | 8 ++--- src/Virt_FilterEntry.c | 46 +++++++++++++++-------------- src/Virt_SettingsDefineCapabilities.c | 16 +++++----- src/Virt_VirtualSystemManagementService.c | 4 +-- 5 files changed, 40 insertions(+), 38 deletions(-) -- 1.7.9.5
SetProperty was called with int instead of uint16_t in a few places. This leads to failures on big-endian systems like s390. Found by runnung cimstest. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/Virt_SettingsDefineCapabilities.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Virt_SettingsDefineCapabilities.c b/src/Virt_SettingsDefineCapabilities.c index 78c128c..fe16e3f 100644 --- a/src/Virt_SettingsDefineCapabilities.c +++ b/src/Virt_SettingsDefineCapabilities.c @@ -1349,7 +1349,7 @@ static CMPIStatus _new_volume_template(const CMPIObjectPath *ref, int ret = 0; struct virt_pool *pool = NULL; CMPIInstance *inst = NULL; - int type = 0; + uint16_t type = 0; const char *name; const char *path; uint16_t alloc = 0; @@ -1712,13 +1712,13 @@ static CMPIStatus disk_pool_template(const CMPIObjectPath *ref, CMPIArray *array; CMPIStatus s = {CMPI_RC_OK, NULL}; const char *path = "/dev/null"; - int type[7] = {DISK_POOL_DIR, - DISK_POOL_FS, - DISK_POOL_NETFS, - DISK_POOL_DISK, - DISK_POOL_ISCSI, - DISK_POOL_LOGICAL, - DISK_POOL_SCSI}; + uint16_t type[7] = {DISK_POOL_DIR, + DISK_POOL_FS, + DISK_POOL_NETFS, + DISK_POOL_DISK, + DISK_POOL_ISCSI, + DISK_POOL_LOGICAL, + DISK_POOL_SCSI}; int pool_types = 7; int i; uint16_t autostart; -- 1.7.9.5
On 10/11/2013 07:47 AM, Viktor Mihajlovski wrote:
SetProperty was called with int instead of uint16_t in a few places. This leads to failures on big-endian systems like s390. Found by runnung cimstest.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/Virt_SettingsDefineCapabilities.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
ACK John
From: Thilo Boehm <tboehm@linux.vnet.ibm.com> A number of CIM properties was set in an endianness-unsafe manner leading to failures on big endian systems. Signed-off-by: Thilo Boehm <tboehm@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/Virt_FilterEntry.c | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c index b7042da..e41b4b6 100644 --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -59,8 +59,8 @@ struct rule_data_t { const char *dstportend; }; -static int octets_from_mac(const char * s, unsigned int *buffer, - unsigned int size) +static int octets_from_mac(const char * s, uint8_t *buffer, + unsigned int size) { unsigned int _buffer[6]; unsigned int i, n = 0; @@ -86,8 +86,8 @@ static int octets_from_mac(const char * s, unsigned int *buffer, return n; } -static int octets_from_ip(const char * s, unsigned int *buffer, - unsigned int size) +static int octets_from_ip(const char * s, uint8_t *buffer, + unsigned int size) { struct in6_addr addr; unsigned int family = 0; @@ -116,7 +116,8 @@ static int octets_from_ip(const char * s, unsigned int *buffer, return n; } -static CMPIArray *octets_to_cmpi(const CMPIBroker *broker, unsigned int *bytes, int size) +static CMPIArray *octets_to_cmpi(const CMPIBroker *broker, uint8_t *bytes, + int size) { CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIArray *array = NULL; @@ -216,7 +217,7 @@ static int convert_action(const char *s) return action; } -static unsigned long convert_protocol_id(const char *s) +static uint16_t convert_protocol_id(const char *s) { enum {NONE = 0, IPV4 = 2048, ARP = 2054, RARP = 32821, IPV6 = 34525} id = NONE; @@ -239,7 +240,7 @@ static void convert_mac_rule_to_instance( CMPIInstance *inst, const CMPIBroker *broker) { - unsigned int bytes[48]; + uint8_t bytes[48]; unsigned int size = 0; CMPIArray *array = NULL; @@ -280,7 +281,7 @@ static void convert_mac_rule_to_instance( (CMPIValue *)&array, CMPI_uint8A); if (rule->var.mac.protocol_id != NULL) { - unsigned long n = convert_protocol_id(rule->var.mac.protocol_id); + uint16_t n = convert_protocol_id(rule->var.mac.protocol_id); /* Unknown protocolid string. Try converting from hexadecimal value */ if (n == 0) @@ -366,18 +367,19 @@ static void convert_ip_rule_to_instance( CMPIInstance *inst, const CMPIBroker *broker) { - unsigned int bytes[48]; + uint8_t bytes[48]; unsigned int size = 0; - unsigned int n = 0; + uint8_t ipver_num = 0; + uint16_t port_num = 0; CMPIArray *array = NULL; struct rule_data_t rule_data; if (strstr(rule->protocol_id, "v6")) - n = 6; + ipver_num = 6; else - n = 4; + ipver_num = 4; - CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&n, CMPI_uint8); + CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&ipver_num, CMPI_uint8); fill_rule_data(rule, &rule_data); @@ -480,27 +482,27 @@ static void convert_ip_rule_to_instance( } if (rule_data.srcportstart) { - n = atoi(rule_data.srcportstart); + port_num = atoi(rule_data.srcportstart); CMSetProperty(inst, "HdrSrcPortStart", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); } if (rule_data.srcportend) { - n = atoi(rule_data.srcportend); + port_num = atoi(rule_data.srcportend); CMSetProperty(inst, "HdrSrcPortEnd", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); } if (rule_data.dstportstart) { - n = atoi(rule_data.dstportstart); + port_num = atoi(rule_data.dstportstart); CMSetProperty(inst, "HdrDestPortStart", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); } if (rule_data.dstportend) { - n = atoi(rule_data.dstportend); + port_num = atoi(rule_data.dstportend); CMSetProperty(inst, "HdrDestPortEnd", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); } } @@ -515,7 +517,7 @@ static CMPIInstance *convert_rule_to_instance( const char *sys_name = NULL; const char *sys_ccname = NULL; const char *basename = NULL; - int action, direction, priority = 0; + uint16_t action, direction, priority = 0; void (*convert_f)(struct acl_rule*, CMPIInstance*, const CMPIBroker*); -- 1.7.9.5
On 10/11/2013 07:47 AM, Viktor Mihajlovski wrote:
From: Thilo Boehm <tboehm@linux.vnet.ibm.com>
A number of CIM properties was set in an endianness-unsafe manner leading to failures on big endian systems.
Signed-off-by: Thilo Boehm <tboehm@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/Virt_FilterEntry.c | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c index b7042da..e41b4b6 100644 --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -59,8 +59,8 @@ struct rule_data_t { const char *dstportend; };
-static int octets_from_mac(const char * s, unsigned int *buffer, - unsigned int size) +static int octets_from_mac(const char * s, uint8_t *buffer, + unsigned int size) { unsigned int _buffer[6]; unsigned int i, n = 0; @@ -86,8 +86,8 @@ static int octets_from_mac(const char * s, unsigned int *buffer, return n; }
-static int octets_from_ip(const char * s, unsigned int *buffer, - unsigned int size) +static int octets_from_ip(const char * s, uint8_t *buffer, + unsigned int size) { struct in6_addr addr; unsigned int family = 0; @@ -116,7 +116,8 @@ static int octets_from_ip(const char * s, unsigned int *buffer, return n; }
-static CMPIArray *octets_to_cmpi(const CMPIBroker *broker, unsigned int *bytes, int size) +static CMPIArray *octets_to_cmpi(const CMPIBroker *broker, uint8_t *bytes, + int size) { CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIArray *array = NULL; @@ -216,7 +217,7 @@ static int convert_action(const char *s) return action; }
-static unsigned long convert_protocol_id(const char *s) +static uint16_t convert_protocol_id(const char *s) { enum {NONE = 0, IPV4 = 2048, ARP = 2054, RARP = 32821, IPV6 = 34525} id = NONE;
@@ -239,7 +240,7 @@ static void convert_mac_rule_to_instance( CMPIInstance *inst, const CMPIBroker *broker) { - unsigned int bytes[48]; + uint8_t bytes[48]; unsigned int size = 0; CMPIArray *array = NULL;
@@ -280,7 +281,7 @@ static void convert_mac_rule_to_instance( (CMPIValue *)&array, CMPI_uint8A);
if (rule->var.mac.protocol_id != NULL) { - unsigned long n = convert_protocol_id(rule->var.mac.protocol_id); + uint16_t n = convert_protocol_id(rule->var.mac.protocol_id);
/* Unknown protocolid string. Try converting from hexadecimal value */ if (n == 0) @@ -366,18 +367,19 @@ static void convert_ip_rule_to_instance( CMPIInstance *inst, const CMPIBroker *broker) { - unsigned int bytes[48]; + uint8_t bytes[48]; unsigned int size = 0; - unsigned int n = 0; + uint8_t ipver_num = 0; + uint16_t port_num = 0; CMPIArray *array = NULL; struct rule_data_t rule_data;
if (strstr(rule->protocol_id, "v6")) - n = 6; + ipver_num = 6; else - n = 4; + ipver_num = 4;
- CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&n, CMPI_uint8); + CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&ipver_num, CMPI_uint8);
fill_rule_data(rule, &rule_data);
@@ -480,27 +482,27 @@ static void convert_ip_rule_to_instance( }
if (rule_data.srcportstart) { - n = atoi(rule_data.srcportstart); + port_num = atoi(rule_data.srcportstart);
Hmm.. technically atoi() returns an 'int'... In libvirt code this would be a call to a function which does a strtoul() and makes sure there's no overflow, etc. However, since this code is full of other situations such as this, I'm "OK" with the way it's done...
CMSetProperty(inst, "HdrSrcPortStart", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); }
if (rule_data.srcportend) { - n = atoi(rule_data.srcportend); + port_num = atoi(rule_data.srcportend); CMSetProperty(inst, "HdrSrcPortEnd", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); }
if (rule_data.dstportstart) { - n = atoi(rule_data.dstportstart); + port_num = atoi(rule_data.dstportstart); CMSetProperty(inst, "HdrDestPortStart", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); }
if (rule_data.dstportend) { - n = atoi(rule_data.dstportend); + port_num = atoi(rule_data.dstportend); CMSetProperty(inst, "HdrDestPortEnd", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); } }
@@ -515,7 +517,7 @@ static CMPIInstance *convert_rule_to_instance( const char *sys_name = NULL; const char *sys_ccname = NULL; const char *basename = NULL; - int action, direction, priority = 0; + uint16_t action, direction, priority = 0;
This is declared an uint16_t here; however, later on it's stored as a signed int16: priority = convert_priority(rule->priority); CMSetProperty(inst, "Priority", (CMPIValue *)&priority, CMPI_sint16); Also in convert_filter_to_instance(), there's some similar oddities: ... int direction = 0, priority; ... CMSetProperty(inst, "Direction", (CMPIValue *)&direction, CMPI_uint16); priority = convert_priority(filter->priority); CMSetProperty(inst, "Priority", (CMPIValue *)&priority, CMPI_sint16); Where 'direction' is a signed value, but being stored unsigned and priority is treated as a signed value. Just let me know which way you'd like to have these adjusted, I'll make the change and then push the series. John
void (*convert_f)(struct acl_rule*, CMPIInstance*, const CMPIBroker*);
On 11/12/2013 10:28 PM, John Ferlan wrote:
On 10/11/2013 07:47 AM, Viktor Mihajlovski wrote:
From: Thilo Boehm <tboehm@linux.vnet.ibm.com>
A number of CIM properties was set in an endianness-unsafe manner leading to failures on big endian systems.
Signed-off-by: Thilo Boehm <tboehm@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/Virt_FilterEntry.c | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c index b7042da..e41b4b6 100644 --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -59,8 +59,8 @@ struct rule_data_t { const char *dstportend; };
-static int octets_from_mac(const char * s, unsigned int *buffer, - unsigned int size) +static int octets_from_mac(const char * s, uint8_t *buffer, + unsigned int size) { unsigned int _buffer[6]; unsigned int i, n = 0; @@ -86,8 +86,8 @@ static int octets_from_mac(const char * s, unsigned int *buffer, return n; }
-static int octets_from_ip(const char * s, unsigned int *buffer, - unsigned int size) +static int octets_from_ip(const char * s, uint8_t *buffer, + unsigned int size) { struct in6_addr addr; unsigned int family = 0; @@ -116,7 +116,8 @@ static int octets_from_ip(const char * s, unsigned int *buffer, return n; }
-static CMPIArray *octets_to_cmpi(const CMPIBroker *broker, unsigned int *bytes, int size) +static CMPIArray *octets_to_cmpi(const CMPIBroker *broker, uint8_t *bytes, + int size) { CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIArray *array = NULL; @@ -216,7 +217,7 @@ static int convert_action(const char *s) return action; }
-static unsigned long convert_protocol_id(const char *s) +static uint16_t convert_protocol_id(const char *s) { enum {NONE = 0, IPV4 = 2048, ARP = 2054, RARP = 32821, IPV6 = 34525} id = NONE;
@@ -239,7 +240,7 @@ static void convert_mac_rule_to_instance( CMPIInstance *inst, const CMPIBroker *broker) { - unsigned int bytes[48]; + uint8_t bytes[48]; unsigned int size = 0; CMPIArray *array = NULL;
@@ -280,7 +281,7 @@ static void convert_mac_rule_to_instance( (CMPIValue *)&array, CMPI_uint8A);
if (rule->var.mac.protocol_id != NULL) { - unsigned long n = convert_protocol_id(rule->var.mac.protocol_id); + uint16_t n = convert_protocol_id(rule->var.mac.protocol_id);
/* Unknown protocolid string. Try converting from hexadecimal value */ if (n == 0) @@ -366,18 +367,19 @@ static void convert_ip_rule_to_instance( CMPIInstance *inst, const CMPIBroker *broker) { - unsigned int bytes[48]; + uint8_t bytes[48]; unsigned int size = 0; - unsigned int n = 0; + uint8_t ipver_num = 0; + uint16_t port_num = 0; CMPIArray *array = NULL; struct rule_data_t rule_data;
if (strstr(rule->protocol_id, "v6")) - n = 6; + ipver_num = 6; else - n = 4; + ipver_num = 4;
- CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&n, CMPI_uint8); + CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&ipver_num, CMPI_uint8);
fill_rule_data(rule, &rule_data);
@@ -480,27 +482,27 @@ static void convert_ip_rule_to_instance( }
if (rule_data.srcportstart) { - n = atoi(rule_data.srcportstart); + port_num = atoi(rule_data.srcportstart);
Hmm.. technically atoi() returns an 'int'... In libvirt code this would be a call to a function which does a strtoul() and makes sure there's no overflow, etc. However, since this code is full of other situations such as this, I'm "OK" with the way it's done... too true ... we have tried to restrict the patch to the big/little- endian fix and not to mix in cleanups.
CMSetProperty(inst, "HdrSrcPortStart", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); }
if (rule_data.srcportend) { - n = atoi(rule_data.srcportend); + port_num = atoi(rule_data.srcportend); CMSetProperty(inst, "HdrSrcPortEnd", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); }
if (rule_data.dstportstart) { - n = atoi(rule_data.dstportstart); + port_num = atoi(rule_data.dstportstart); CMSetProperty(inst, "HdrDestPortStart", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); }
if (rule_data.dstportend) { - n = atoi(rule_data.dstportend); + port_num = atoi(rule_data.dstportend); CMSetProperty(inst, "HdrDestPortEnd", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); } }
@@ -515,7 +517,7 @@ static CMPIInstance *convert_rule_to_instance( const char *sys_name = NULL; const char *sys_ccname = NULL; const char *basename = NULL; - int action, direction, priority = 0; + uint16_t action, direction, priority = 0;
This is declared an uint16_t here; however, later on it's stored as a signed int16:
priority = convert_priority(rule->priority); CMSetProperty(inst, "Priority", (CMPIValue *)&priority, CMPI_sint16); hm ... collateral damage. The "Priority" property is indeed a sint16 (schema/FilterEntry.mof), "Direction" and "Action" are uint16's.
Also in convert_filter_to_instance(), there's some similar oddities:
... int direction = 0, priority; oversight ... should be uint16_t direction = 0; int16_t priority; ... CMSetProperty(inst, "Direction", (CMPIValue *)&direction, CMPI_uint16);
priority = convert_priority(filter->priority); further, we need to change the return type of all the conver_xxx functions ...
In my opinion this warrants a V2 patch ... do you prefer a resend of entire series or just this single patch? -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
From: Thilo Boehm <tboehm@linux.vnet.ibm.com> A number of CIM properties was set in an endianness-unsafe manner leading to failures on big endian systems. Signed-off-by: Thilo Boehm <tboehm@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/Virt_FilterEntry.c | 53 +++++++++++++++++++++++++----------------------- src/Virt_FilterEntry.h | 2 +- src/Virt_FilterList.c | 3 ++- 3 files changed, 31 insertions(+), 27 deletions(-) V2 Changes: - The FilterEntry "Priority" property is a signed 16bit value, fix the variable declaration accordingly - Change the convert_xxx functions to return a properly typed int value diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c index b7042da..ab4a512 100644 --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -59,8 +59,8 @@ struct rule_data_t { const char *dstportend; }; -static int octets_from_mac(const char * s, unsigned int *buffer, - unsigned int size) +static int octets_from_mac(const char * s, uint8_t *buffer, + unsigned int size) { unsigned int _buffer[6]; unsigned int i, n = 0; @@ -86,8 +86,8 @@ static int octets_from_mac(const char * s, unsigned int *buffer, return n; } -static int octets_from_ip(const char * s, unsigned int *buffer, - unsigned int size) +static int octets_from_ip(const char * s, uint8_t *buffer, + unsigned int size) { struct in6_addr addr; unsigned int family = 0; @@ -116,7 +116,8 @@ static int octets_from_ip(const char * s, unsigned int *buffer, return n; } -static CMPIArray *octets_to_cmpi(const CMPIBroker *broker, unsigned int *bytes, int size) +static CMPIArray *octets_to_cmpi(const CMPIBroker *broker, uint8_t *bytes, + int size) { CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIArray *array = NULL; @@ -173,7 +174,7 @@ static char *cidr_to_str(const char *cidr) return ret; } -static int convert_direction(const char *s) +static uint16_t convert_direction(const char *s) { enum {NOT_APPLICABLE, INPUT, OUTPUT, BOTH} direction = NOT_APPLICABLE; @@ -189,7 +190,7 @@ static int convert_direction(const char *s) return direction; } -int convert_priority(const char *s) +int16_t convert_priority(const char *s) { if (s == NULL) return 0; @@ -197,7 +198,7 @@ int convert_priority(const char *s) return atoi(s); } -static int convert_action(const char *s) +static uint16_t convert_action(const char *s) { enum {NONE=0, ACCEPT, DENY, REJECT, RETURN, CONTINUE} action = NONE; @@ -216,7 +217,7 @@ static int convert_action(const char *s) return action; } -static unsigned long convert_protocol_id(const char *s) +static uint16_t convert_protocol_id(const char *s) { enum {NONE = 0, IPV4 = 2048, ARP = 2054, RARP = 32821, IPV6 = 34525} id = NONE; @@ -239,7 +240,7 @@ static void convert_mac_rule_to_instance( CMPIInstance *inst, const CMPIBroker *broker) { - unsigned int bytes[48]; + uint8_t bytes[48]; unsigned int size = 0; CMPIArray *array = NULL; @@ -280,7 +281,7 @@ static void convert_mac_rule_to_instance( (CMPIValue *)&array, CMPI_uint8A); if (rule->var.mac.protocol_id != NULL) { - unsigned long n = convert_protocol_id(rule->var.mac.protocol_id); + uint16_t n = convert_protocol_id(rule->var.mac.protocol_id); /* Unknown protocolid string. Try converting from hexadecimal value */ if (n == 0) @@ -366,18 +367,19 @@ static void convert_ip_rule_to_instance( CMPIInstance *inst, const CMPIBroker *broker) { - unsigned int bytes[48]; + uint8_t bytes[48]; unsigned int size = 0; - unsigned int n = 0; + uint8_t ipver_num = 0; + uint16_t port_num = 0; CMPIArray *array = NULL; struct rule_data_t rule_data; if (strstr(rule->protocol_id, "v6")) - n = 6; + ipver_num = 6; else - n = 4; + ipver_num = 4; - CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&n, CMPI_uint8); + CMSetProperty(inst, "HdrIPVersion",(CMPIValue *)&ipver_num, CMPI_uint8); fill_rule_data(rule, &rule_data); @@ -480,27 +482,27 @@ static void convert_ip_rule_to_instance( } if (rule_data.srcportstart) { - n = atoi(rule_data.srcportstart); + port_num = atoi(rule_data.srcportstart); CMSetProperty(inst, "HdrSrcPortStart", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); } if (rule_data.srcportend) { - n = atoi(rule_data.srcportend); + port_num = atoi(rule_data.srcportend); CMSetProperty(inst, "HdrSrcPortEnd", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); } if (rule_data.dstportstart) { - n = atoi(rule_data.dstportstart); + port_num = atoi(rule_data.dstportstart); CMSetProperty(inst, "HdrDestPortStart", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); } if (rule_data.dstportend) { - n = atoi(rule_data.dstportend); + port_num = atoi(rule_data.dstportend); CMSetProperty(inst, "HdrDestPortEnd", - (CMPIValue *)&n, CMPI_uint16); + (CMPIValue *)&port_num, CMPI_uint16); } } @@ -515,7 +517,8 @@ static CMPIInstance *convert_rule_to_instance( const char *sys_name = NULL; const char *sys_ccname = NULL; const char *basename = NULL; - int action, direction, priority = 0; + uint16_t action, direction; + int16_t priority; void (*convert_f)(struct acl_rule*, CMPIInstance*, const CMPIBroker*); diff --git a/src/Virt_FilterEntry.h b/src/Virt_FilterEntry.h index 5057fb0..0589aa0 100644 --- a/src/Virt_FilterEntry.h +++ b/src/Virt_FilterEntry.h @@ -77,7 +77,7 @@ CMPIStatus instance_from_rule( * * @param s A pointer to a string representing the priority */ -int convert_priority(const char *s); +int16_t convert_priority(const char *s); #endif /* diff --git a/src/Virt_FilterList.c b/src/Virt_FilterList.c index 7026d8b..b248004 100644 --- a/src/Virt_FilterList.c +++ b/src/Virt_FilterList.c @@ -45,7 +45,8 @@ static CMPIInstance *convert_filter_to_instance( CMPIInstance *inst = NULL; const char *sys_name = NULL; const char *sys_ccname = NULL; - int direction = 0, priority; + uint16_t direction = 0; + int16_t priority; inst = get_typed_instance(broker, CLASSNAME(reference), -- 1.7.9.5
On 11/13/2013 01:07 PM, Viktor Mihajlovski wrote:
From: Thilo Boehm <tboehm@linux.vnet.ibm.com>
A number of CIM properties was set in an endianness-unsafe manner leading to failures on big endian systems.
Signed-off-by: Thilo Boehm <tboehm@linux.vnet.ibm.com> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- src/Virt_FilterEntry.c | 53 +++++++++++++++++++++++++----------------------- src/Virt_FilterEntry.h | 2 +- src/Virt_FilterList.c | 3 ++- 3 files changed, 31 insertions(+), 27 deletions(-)
V2 Changes: - The FilterEntry "Priority" property is a signed 16bit value, fix the variable declaration accordingly - Change the convert_xxx functions to return a properly typed int value
ACK and pushed too John
From: Thilo Boehm <tboehm@linux.vnet.ibm.com> The properties for the on_xxx actions must be 16-bit values in order to avoid failures on big endian systems. Signed-off-by: Thilo Boehm <tboehm@linux.vnet.ibm.com> Reviewed-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- libxkutil/device_parsing.c | 2 +- libxkutil/device_parsing.h | 8 ++++---- src/Virt_VirtualSystemManagementService.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index aecca4c..0636864 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -1489,7 +1489,7 @@ static int parse_features(struct domain *dominfo, xmlNode *features) return 1; } -static void set_action(int *val, xmlNode *child) +static void set_action(uint16_t *val, xmlNode *child) { char *action = (char *)xmlNodeGetContent(child); diff --git a/libxkutil/device_parsing.h b/libxkutil/device_parsing.h index 2803d6a..4beac5c 100644 --- a/libxkutil/device_parsing.h +++ b/libxkutil/device_parsing.h @@ -205,7 +205,7 @@ struct domain { bool acpi; bool apic; bool pae; - int autostrt; + uint16_t autostrt; union { struct pv_os_info pv; @@ -213,9 +213,9 @@ struct domain { struct lxc_os_info lxc; } os_info; - int on_poweroff; - int on_reboot; - int on_crash; + uint16_t on_poweroff; + uint16_t on_reboot; + uint16_t on_crash; struct virt_device *dev_graphics; int dev_graphics_ct; diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index d51f230..89221bc 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -719,13 +719,13 @@ static int vssd_to_domain(CMPIInstance *inst, if (ret != CMPI_RC_OK) tmp = 0; - domain->on_poweroff = (int)tmp; + domain->on_poweroff = tmp; ret = cu_get_u16_prop(inst, "AutomaticRecoveryAction", &tmp); if (ret != CMPI_RC_OK) tmp = CIM_VSSD_RECOVERY_NONE; - domain->on_crash = (int)tmp; + domain->on_crash = tmp; if (cu_get_bool_prop(inst, "IsFullVirt", &fullvirt) != CMPI_RC_OK) fullvirt = false; -- 1.7.9.5
On 10/11/2013 07:47 AM, Viktor Mihajlovski wrote:
From: Thilo Boehm <tboehm@linux.vnet.ibm.com>
The properties for the on_xxx actions must be 16-bit values in order to avoid failures on big endian systems.
Signed-off-by: Thilo Boehm <tboehm@linux.vnet.ibm.com> Reviewed-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- libxkutil/device_parsing.c | 2 +- libxkutil/device_parsing.h | 8 ++++---- src/Virt_VirtualSystemManagementService.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-)
ACK John
If cleanup_virt_device is called twice (e.g. during modify resource) a double free can occur because only the dev substructure has been memset to zero. Now zeroing the entire structure. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- libxkutil/device_parsing.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index 0636864..076bec0 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -308,7 +308,7 @@ void cleanup_virt_device(struct virt_device *dev) free(dev->id); - memset(&dev->dev, 0, sizeof(dev->dev)); + memset(dev, 0, sizeof(*dev)); } void cleanup_virt_devices(struct virt_device **_devs, int count) -- 1.7.9.5
On 10/11/2013 07:47 AM, Viktor Mihajlovski wrote:
If cleanup_virt_device is called twice (e.g. during modify resource) a double free can occur because only the dev substructure has been memset to zero. Now zeroing the entire structure.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> --- libxkutil/device_parsing.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK John
participants (2)
-
John Ferlan -
Viktor Mihajlovski