6:47 a.m.
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
6:47 a.m.
New subject: [PATCH 1/4] VSDC: Fix endianess issues
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(a)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
3:13 p.m.
New subject: [PATCH 1/4] VSDC: Fix endianess issues
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(a)linux.vnet.ibm.com>
---
src/Virt_SettingsDefineCapabilities.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
ACK
John
6:47 a.m.
New subject: [PATCH 2/4] FilterEntry: Fix endianness issues
From: Thilo Boehm <tboehm(a)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(a)linux.vnet.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov(a)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
3:28 p.m.
New subject: [PATCH 2/4] FilterEntry: Fix endianness issues
On 10/11/2013 07:47 AM, Viktor Mihajlovski wrote:
From: Thilo Boehm <tboehm(a)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(a)linux.vnet.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov(a)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*);
10:51 a.m.
New subject: [PATCH 2/4] FilterEntry: Fix endianness issues
On 11/12/2013 10:28 PM, John Ferlan wrote:
On 10/11/2013 07:47 AM, Viktor Mihajlovski wrote:
> From: Thilo Boehm <tboehm(a)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(a)linux.vnet.ibm.com>
> Signed-off-by: Viktor Mihajlovski <mihajlov(a)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
12:07 p.m.
New subject: [PATCHv2 2/4] FilterEntry: Fix endianness issues
From: Thilo Boehm <tboehm(a)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(a)linux.vnet.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov(a)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
12:34 p.m.
New subject: [PATCHv2 2/4] FilterEntry: Fix endianness issues
On 11/13/2013 01:07 PM, Viktor Mihajlovski wrote:
From: Thilo Boehm <tboehm(a)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(a)linux.vnet.ibm.com>
Signed-off-by: Viktor Mihajlovski <mihajlov(a)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
6:47 a.m.
New subject: [PATCH 3/4] VSSM: Fix endianness issue in domain properties
From: Thilo Boehm <tboehm(a)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(a)linux.vnet.ibm.com>
Reviewed-by: Viktor Mihajlovski <mihajlov(a)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
3:46 p.m.
New subject: [PATCH 3/4] VSSM: Fix endianness issue in domain properties
On 10/11/2013 07:47 AM, Viktor Mihajlovski wrote:
From: Thilo Boehm <tboehm(a)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(a)linux.vnet.ibm.com>
Reviewed-by: Viktor Mihajlovski <mihajlov(a)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
6:47 a.m.
New subject: [PATCH 4/4] libxkutil: clean entire device structure to avoid memory corruption
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(a)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
4:16 p.m.
New subject: [PATCH 4/4] libxkutil: clean entire device structure to avoid memory corruption
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(a)linux.vnet.ibm.com>
---
libxkutil/device_parsing.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK
John
4056
days inactive
4089
days old
12 comments
2 participants
participants (2)
-
John Ferlan -
Viktor Mihajlovski