[Libvirt-cim] [PATCH 2/4] FilterEntry: Fix endianness issues

John Ferlan jferlan at redhat.com
Tue Nov 12 21:28:45 UTC 2013


On 10/11/2013 07:47 AM, Viktor Mihajlovski wrote:
> From: Thilo Boehm <tboehm at 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 at linux.vnet.ibm.com>
> Signed-off-by: Viktor Mihajlovski <mihajlov at 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*);
>  
> 




More information about the Libvirt-cim mailing list