power: fix buffer overruns

A previous change removed the limit of 64 cores by
moving away from 64-bit masks to char arrays. However
this left a buffer overrun issue, where the max channels
was defined as 64, and max cores was defined as 256. These
should all be consistently set to RTE_MAX_LCORE.

The #defines being removed are CHANNEL_CMDS_MAX_CPUS,
CHANNEL_CMDS_MAX_CHANNELS, POWER_MGR_MAX_CPUS, and
CHANNEL_CMDS_MAX_VM_CHANNELS, and are being replaced
with RTE_MAX_LCORE for consistency and simplicity.

Coverity issue: 337672, 337673, 337678
Fixes: fd73630e95c1 ("examples/power: change 64-bit masks to arrays")
Cc: stable@dpdk.org
Signed-off-by: David Hunt <david.hunt@intel.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

diff --git a/examples/vm_power_manager/channel_manager.c b/examples/vm_power_manager/channel_manager.c
index 0187f79..0846817 100644 (file)
--- a/examples/vm_power_manager/channel_manager.c
+++ b/examples/vm_power_manager/channel_manager.c
@@ -50,9 +50,9 @@ static bool global_hypervisor_available;
  */
 struct virtual_machine_info {
        char name[CHANNEL_MGR_MAX_NAME_LEN];
-       uint16_t pcpu_map[CHANNEL_CMDS_MAX_CPUS];
-       struct channel_info *channels[CHANNEL_CMDS_MAX_VM_CHANNELS];
-       char channel_mask[POWER_MGR_MAX_CPUS];
+       uint16_t pcpu_map[RTE_MAX_LCORE];
+       struct channel_info *channels[RTE_MAX_LCORE];
+       char channel_mask[RTE_MAX_LCORE];
        uint8_t num_channels;
        enum vm_status status;
        virDomainPtr domainPtr;
@@ -81,7 +81,7 @@ update_pcpus_mask(struct virtual_machine_info *vm_info)
        unsigned i, j;
        int n_vcpus;
 
-       memset(global_cpumaps, 0, CHANNEL_CMDS_MAX_CPUS*global_maplen);
+       memset(global_cpumaps, 0, RTE_MAX_LCORE*global_maplen);
 
        if (!virDomainIsActive(vm_info->domainPtr)) {
                n_vcpus = virDomainGetVcpuPinInfo(vm_info->domainPtr,
@@ -96,21 +96,21 @@ update_pcpus_mask(struct virtual_machine_info *vm_info)
        }
 
        memset(global_vircpuinfo, 0, sizeof(*global_vircpuinfo)*
-                       CHANNEL_CMDS_MAX_CPUS);
+                       RTE_MAX_LCORE);
 
        cpuinfo = global_vircpuinfo;
 
        n_vcpus = virDomainGetVcpus(vm_info->domainPtr, cpuinfo,
-                       CHANNEL_CMDS_MAX_CPUS, global_cpumaps, global_maplen);
+                       RTE_MAX_LCORE, global_cpumaps, global_maplen);
        if (n_vcpus < 0) {
                RTE_LOG(ERR, CHANNEL_MANAGER, "Error getting vCPU info for "
                                "active VM '%s'\n", vm_info->name);
                return -1;
        }
 update_pcpus:
-       if (n_vcpus >= CHANNEL_CMDS_MAX_CPUS) {
+       if (n_vcpus >= RTE_MAX_LCORE) {
                RTE_LOG(ERR, CHANNEL_MANAGER, "Number of vCPUS(%u) is out of range "
-                               "0...%d\n", n_vcpus, CHANNEL_CMDS_MAX_CPUS-1);
+                               "0...%d\n", n_vcpus, RTE_MAX_LCORE-1);
                return -1;
        }
        if (n_vcpus != vm_info->info.nrVirtCpu) {
@@ -138,9 +138,9 @@ set_pcpu(char *vm_name, unsigned int vcpu, unsigned int pcpu)
        int flags = VIR_DOMAIN_AFFECT_LIVE|VIR_DOMAIN_AFFECT_CONFIG;
        struct virtual_machine_info *vm_info;
 
-       if (vcpu >= CHANNEL_CMDS_MAX_CPUS) {
+       if (vcpu >= RTE_MAX_LCORE) {
                RTE_LOG(ERR, CHANNEL_MANAGER, "vCPU(%u) exceeds max allowable(%d)\n",
-                               vcpu, CHANNEL_CMDS_MAX_CPUS-1);
+                               vcpu, RTE_MAX_LCORE-1);
                return -1;
        }
 
@@ -162,7 +162,7 @@ set_pcpu(char *vm_name, unsigned int vcpu, unsigned int pcpu)
                                "vCPUs(%u)\n", vcpu, vm_info->info.nrVirtCpu);
                return -1;
        }
-       memset(global_cpumaps, 0 , CHANNEL_CMDS_MAX_CPUS * global_maplen);
+       memset(global_cpumaps, 0, RTE_MAX_LCORE * global_maplen);
 
        VIR_USE_CPU(global_cpumaps, pcpu);
 
@@ -436,10 +436,10 @@ add_all_channels(const char *vm_name)
                                        dir->d_name);
                        continue;
                }
-               if (channel_num >= CHANNEL_CMDS_MAX_VM_CHANNELS) {
+               if (channel_num >= RTE_MAX_LCORE) {
                        RTE_LOG(WARNING, CHANNEL_MANAGER, "Channel number(%u) is "
                                        "greater than max allowable: %d, skipping '%s%s'\n",
-                                       channel_num, CHANNEL_CMDS_MAX_VM_CHANNELS-1,
+                                       channel_num, RTE_MAX_LCORE-1,
                                        CHANNEL_MGR_SOCKET_PATH, dir->d_name);
                        continue;
                }
@@ -495,10 +495,10 @@ add_channels(const char *vm_name, unsigned *channel_list,
 
        for (i = 0; i < len_channel_list; i++) {
 
-               if (channel_list[i] >= CHANNEL_CMDS_MAX_VM_CHANNELS) {
+               if (channel_list[i] >= RTE_MAX_LCORE) {
                        RTE_LOG(INFO, CHANNEL_MANAGER, "Channel(%u) is out of range "
                                                        "0...%d\n", channel_list[i],
-                                                       CHANNEL_CMDS_MAX_VM_CHANNELS-1);
+                                                       RTE_MAX_LCORE-1);
                        continue;
                }
                if (channel_exists(vm_info, channel_list[i])) {
@@ -598,7 +598,7 @@ set_channel_status_all(const char *vm_name, enum channel_status status)
 {
        struct virtual_machine_info *vm_info;
        unsigned i;
-       char mask[POWER_MGR_MAX_CPUS];
+       char mask[RTE_MAX_LCORE];
        int num_channels_changed = 0;
 
        if (!(status == CHANNEL_MGR_CHANNEL_CONNECTED ||
@@ -614,8 +614,8 @@ set_channel_status_all(const char *vm_name, enum channel_status status)
        }
 
        rte_spinlock_lock(&(vm_info->config_spinlock));
-       memcpy(mask, (char *)vm_info->channel_mask, POWER_MGR_MAX_CPUS);
-       for (i = 0; i < POWER_MGR_MAX_CPUS; i++) {
+       memcpy(mask, (char *)vm_info->channel_mask, RTE_MAX_LCORE);
+       for (i = 0; i < RTE_MAX_LCORE; i++) {
                if (mask[i] != 1)
                        continue;
                vm_info->channels[i]->status = status;
@@ -672,7 +672,7 @@ get_all_vm(int *num_vm, int *num_vcpu)
        if (!global_hypervisor_available)
                return;
 
-       memset(global_cpumaps, 0, CHANNEL_CMDS_MAX_CPUS*global_maplen);
+       memset(global_cpumaps, 0, RTE_MAX_LCORE*global_maplen);
        if (virNodeGetInfo(global_vir_conn_ptr, &node_info)) {
                RTE_LOG(ERR, CHANNEL_MANAGER, "Unable to retrieve node Info\n");
                return;
@@ -725,7 +725,7 @@ get_info_vm(const char *vm_name, struct vm_info *info)
 {
        struct virtual_machine_info *vm_info;
        unsigned i, channel_num = 0;
-       char mask[POWER_MGR_MAX_CPUS];
+       char mask[RTE_MAX_LCORE];
 
        vm_info = find_domain_by_name(vm_name);
        if (vm_info == NULL) {
@@ -738,8 +738,8 @@ get_info_vm(const char *vm_name, struct vm_info *info)
 
        rte_spinlock_lock(&(vm_info->config_spinlock));
 
-       memcpy(mask, (char *)vm_info->channel_mask, POWER_MGR_MAX_CPUS);
-       for (i = 0; i < POWER_MGR_MAX_CPUS; i++) {
+       memcpy(mask, (char *)vm_info->channel_mask, RTE_MAX_LCORE);
+       for (i = 0; i < RTE_MAX_LCORE; i++) {
                if (mask[i] != 1)
                        continue;
                info->channels[channel_num].channel_num = i;
@@ -803,17 +803,17 @@ add_vm(const char *vm_name)
                rte_free(new_domain);
                return -1;
        }
-       if (new_domain->info.nrVirtCpu > CHANNEL_CMDS_MAX_CPUS) {
+       if (new_domain->info.nrVirtCpu > RTE_MAX_LCORE) {
                RTE_LOG(ERR, CHANNEL_MANAGER, "Error the number of virtual CPUs(%u) is "
                                "greater than allowable(%d)\n", new_domain->info.nrVirtCpu,
-                               CHANNEL_CMDS_MAX_CPUS);
+                               RTE_MAX_LCORE);
                rte_free(new_domain);
                return -1;
        }
 
-       for (i = 0; i < CHANNEL_CMDS_MAX_CPUS; i++) {
+       for (i = 0; i < RTE_MAX_LCORE; i++)
                new_domain->pcpu_map[i] = 0;
-       }
+
        if (update_pcpus_mask(new_domain) < 0) {
                RTE_LOG(ERR, CHANNEL_MANAGER, "Error getting physical CPU pinning\n");
                rte_free(new_domain);
@@ -821,7 +821,7 @@ add_vm(const char *vm_name)
        }
        strncpy(new_domain->name, vm_name, sizeof(new_domain->name));
        new_domain->name[sizeof(new_domain->name) - 1] = '\0';
-       memset(new_domain->channel_mask, 0, POWER_MGR_MAX_CPUS);
+       memset(new_domain->channel_mask, 0, RTE_MAX_LCORE);
        new_domain->num_channels = 0;
 
        if (!virDomainIsActive(dom_ptr))
@@ -896,17 +896,17 @@ channel_manager_init(const char *path __rte_unused)
        } else {
                global_hypervisor_available = 1;
 
-               global_maplen = VIR_CPU_MAPLEN(CHANNEL_CMDS_MAX_CPUS);
+               global_maplen = VIR_CPU_MAPLEN(RTE_MAX_LCORE);
 
                global_vircpuinfo = rte_zmalloc(NULL,
                                sizeof(*global_vircpuinfo) *
-                               CHANNEL_CMDS_MAX_CPUS, RTE_CACHE_LINE_SIZE);
+                               RTE_MAX_LCORE, RTE_CACHE_LINE_SIZE);
                if (global_vircpuinfo == NULL) {
                        RTE_LOG(ERR, CHANNEL_MANAGER, "Error allocating memory for CPU Info\n");
                        goto error;
                }
                global_cpumaps = rte_zmalloc(NULL,
-                               CHANNEL_CMDS_MAX_CPUS * global_maplen,
+                               RTE_MAX_LCORE * global_maplen,
                                RTE_CACHE_LINE_SIZE);
                if (global_cpumaps == NULL)
                        goto error;
@@ -920,12 +920,12 @@ channel_manager_init(const char *path __rte_unused)
 
 
 
-       if (global_n_host_cpus > CHANNEL_CMDS_MAX_CPUS) {
+       if (global_n_host_cpus > RTE_MAX_LCORE) {
                RTE_LOG(WARNING, CHANNEL_MANAGER, "The number of host CPUs(%u) exceeds the "
                                "maximum of %u. No cores over %u should be used.\n",
-                               global_n_host_cpus, CHANNEL_CMDS_MAX_CPUS,
-                               CHANNEL_CMDS_MAX_CPUS - 1);
-               global_n_host_cpus = CHANNEL_CMDS_MAX_CPUS;
+                               global_n_host_cpus, RTE_MAX_LCORE,
+                               RTE_MAX_LCORE - 1);
+               global_n_host_cpus = RTE_MAX_LCORE;
        }
 
        return 0;
@@ -939,15 +939,15 @@ void
 channel_manager_exit(void)
 {
        unsigned i;
-       char mask[POWER_MGR_MAX_CPUS];
+       char mask[RTE_MAX_LCORE];
        struct virtual_machine_info *vm_info;
 
        LIST_FOREACH(vm_info, &vm_list_head, vms_info) {
 
                rte_spinlock_lock(&(vm_info->config_spinlock));
 
-               memcpy(mask, (char *)vm_info->channel_mask, POWER_MGR_MAX_CPUS);
-               for (i = 0; i < POWER_MGR_MAX_CPUS; i++) {
+               memcpy(mask, (char *)vm_info->channel_mask, RTE_MAX_LCORE);
+               for (i = 0; i < RTE_MAX_LCORE; i++) {
                        if (mask[i] != 1)
                                continue;
                        remove_channel_from_monitor(

diff --git a/examples/vm_power_manager/channel_manager.h b/examples/vm_power_manager/channel_manager.h
index c3cdce4..251d216 100644 (file)
--- a/examples/vm_power_manager/channel_manager.h
+++ b/examples/vm_power_manager/channel_manager.h
@@ -13,15 +13,9 @@ extern "C" {
 #include <sys/un.h>
 #include <rte_atomic.h>
 
-/* Maximum number of CPUs */
-#define CHANNEL_CMDS_MAX_CPUS        256
-
 /* Maximum name length including '\0' terminator */
 #define CHANNEL_MGR_MAX_NAME_LEN    64
 
-/* Maximum number of channels to each Virtual Machine */
-#define CHANNEL_MGR_MAX_CHANNELS    256
-
 /* Hypervisor Path for libvirt(qemu/KVM) */
 #define CHANNEL_MGR_DEFAULT_HV_PATH "qemu:///system"
 
@@ -78,9 +72,9 @@ struct channel_info {
 struct vm_info {
        char name[CHANNEL_MGR_MAX_NAME_LEN];          /**< VM name */
        enum vm_status status;                        /**< libvirt status */
-       uint16_t pcpu_map[CHANNEL_CMDS_MAX_CPUS];     /**< pCPU map to vCPU */
+       uint16_t pcpu_map[RTE_MAX_LCORE];             /**< pCPU map to vCPU */
        unsigned num_vcpus;                           /**< number of vCPUS */
-       struct channel_info channels[CHANNEL_MGR_MAX_CHANNELS]; /**< Array of channel_info */
+       struct channel_info channels[RTE_MAX_LCORE];  /**< channel_info array */
        unsigned num_channels;                        /**< Number of channels */
 };
 

diff --git a/examples/vm_power_manager/power_manager.c b/examples/vm_power_manager/power_manager.c
index aef8326..9553455 100644 (file)
--- a/examples/vm_power_manager/power_manager.c
+++ b/examples/vm_power_manager/power_manager.c
@@ -39,7 +39,7 @@ struct freq_info {
        unsigned num_freqs;
 } __rte_cache_aligned;
 
-static struct freq_info global_core_freq_info[POWER_MGR_MAX_CPUS];
+static struct freq_info global_core_freq_info[RTE_MAX_LCORE];
 
 struct core_info ci;
 
@@ -92,8 +92,8 @@ power_manager_init(void)
                return -1;
        }
 
-       if (ci->core_count > POWER_MGR_MAX_CPUS)
-               max_core_num = POWER_MGR_MAX_CPUS;
+       if (ci->core_count > RTE_MAX_LCORE)
+               max_core_num = RTE_MAX_LCORE;
        else
                max_core_num = ci->core_count;
 
@@ -132,9 +132,9 @@ power_manager_get_current_frequency(unsigned core_num)
 {
        uint32_t freq, index;
 
-       if (core_num >= POWER_MGR_MAX_CPUS) {
+       if (core_num >= RTE_MAX_LCORE) {
                RTE_LOG(ERR, POWER_MANAGER, "Core(%u) is out of range 0...%d\n",
-                               core_num, POWER_MGR_MAX_CPUS-1);
+                               core_num, RTE_MAX_LCORE-1);
                return -1;
        }
        if (!(ci.cd[core_num].global_enabled_cpus))
@@ -143,7 +143,7 @@ power_manager_get_current_frequency(unsigned core_num)
        rte_spinlock_lock(&global_core_freq_info[core_num].power_sl);
        index = rte_power_get_freq(core_num);
        rte_spinlock_unlock(&global_core_freq_info[core_num].power_sl);
-       if (index >= POWER_MGR_MAX_CPUS)
+       if (index >= RTE_MAX_LCORE)
                freq = 0;
        else
                freq = global_core_freq_info[core_num].freqs[index];
@@ -166,8 +166,8 @@ power_manager_exit(void)
                return -1;
        }
 
-       if (ci->core_count > POWER_MGR_MAX_CPUS)
-               max_core_num = POWER_MGR_MAX_CPUS;
+       if (ci->core_count > RTE_MAX_LCORE)
+               max_core_num = RTE_MAX_LCORE;
        else
                max_core_num = ci->core_count;
 
@@ -246,7 +246,7 @@ power_manager_scale_core_med(unsigned int core_num)
        struct core_info *ci;
 
        ci = get_core_info();
-       if (core_num >= POWER_MGR_MAX_CPUS)
+       if (core_num >= RTE_MAX_LCORE)
                return -1;
        if (!(ci->cd[core_num].global_enabled_cpus))
                return -1;

diff --git a/examples/vm_power_manager/power_manager.h b/examples/vm_power_manager/power_manager.h
index c367384..e81a60a 100644 (file)
--- a/examples/vm_power_manager/power_manager.h
+++ b/examples/vm_power_manager/power_manager.h
@@ -32,8 +32,6 @@ core_info_init(void);
 
 #define RTE_LOGTYPE_POWER_MANAGER RTE_LOGTYPE_USER1
 
-/* Maximum number of CPUS to manage */
-#define POWER_MGR_MAX_CPUS 256
 /**
  * Initialize power management.
  * Initializes resources and verifies the number of CPUs on the system.

diff --git a/examples/vm_power_manager/vm_power_cli.c b/examples/vm_power_manager/vm_power_cli.c
index 41e89ff..89b000d 100644 (file)
--- a/examples/vm_power_manager/vm_power_cli.c
+++ b/examples/vm_power_manager/vm_power_cli.c
@@ -225,7 +225,7 @@ cmd_channels_op_parsed(void *parsed_result, struct cmdline *cl,
 {
        unsigned num_channels = 0, channel_num, i;
        int channels_added;
-       unsigned channel_list[CHANNEL_CMDS_MAX_VM_CHANNELS];
+       unsigned int channel_list[RTE_MAX_LCORE];
        char *token, *remaining, *tail_ptr;
        struct cmd_channels_op_result *res = parsed_result;
 
@@ -249,10 +249,10 @@ cmd_channels_op_parsed(void *parsed_result, struct cmdline *cl,
                if ((errno != 0) || tail_ptr == NULL || (*tail_ptr != '\0'))
                        break;
 
-               if (channel_num == CHANNEL_CMDS_MAX_VM_CHANNELS) {
+               if (channel_num == RTE_MAX_LCORE) {
                        cmdline_printf(cl, "Channel number '%u' exceeds the maximum number "
                                        "of allowable channels(%u) for VM '%s'\n", channel_num,
-                                       CHANNEL_CMDS_MAX_VM_CHANNELS, res->vm_name);
+                                       RTE_MAX_LCORE, res->vm_name);
                        return;
                }
                channel_list[num_channels++] = channel_num;
@@ -306,7 +306,7 @@ cmd_channels_status_op_parsed(void *parsed_result, struct cmdline *cl,
 {
        unsigned num_channels = 0, channel_num;
        int changed;
-       unsigned channel_list[CHANNEL_CMDS_MAX_VM_CHANNELS];
+       unsigned int channel_list[RTE_MAX_LCORE];
        char *token, *remaining, *tail_ptr;
        struct cmd_channels_status_op_result *res = parsed_result;
        enum channel_status status;
@@ -334,10 +334,10 @@ cmd_channels_status_op_parsed(void *parsed_result, struct cmdline *cl,
                if ((errno != 0) || tail_ptr == NULL || (*tail_ptr != '\0'))
                        break;
 
-               if (channel_num == CHANNEL_CMDS_MAX_VM_CHANNELS) {
+               if (channel_num == RTE_MAX_LCORE) {
                        cmdline_printf(cl, "%u exceeds the maximum number of allowable "
                                        "channels(%u) for VM '%s'\n", channel_num,
-                                       CHANNEL_CMDS_MAX_VM_CHANNELS, res->vm_name);
+                                       RTE_MAX_LCORE, res->vm_name);
                        return;
                }
                channel_list[num_channels++] = channel_num;

diff --git a/lib/librte_power/channel_commands.h b/lib/librte_power/channel_commands.h
index e7b93a7..eca8ff7 100644 (file)
--- a/lib/librte_power/channel_commands.h
+++ b/lib/librte_power/channel_commands.h
@@ -12,9 +12,6 @@ extern "C" {
 #include <stdint.h>
 #include <stdbool.h>
 
-/* Maximum number of channels per VM */
-#define CHANNEL_CMDS_MAX_VM_CHANNELS 64
-
 /* Valid Commands */
 #define CPU_POWER               1
 #define CPU_POWER_CONNECT       2

diff --git a/lib/librte_power/power_kvm_vm.c b/lib/librte_power/power_kvm_vm.c
index 20659b7..277ebbe 100644 (file)
--- a/lib/librte_power/power_kvm_vm.c
+++ b/lib/librte_power/power_kvm_vm.c
@@ -13,15 +13,15 @@
 
 #define FD_PATH "/dev/virtio-ports/virtio.serial.port.poweragent"
 
-static struct channel_packet pkt[CHANNEL_CMDS_MAX_VM_CHANNELS];
+static struct channel_packet pkt[RTE_MAX_LCORE];
 
 
 int
 power_kvm_vm_init(unsigned int lcore_id)
 {
-       if (lcore_id >= CHANNEL_CMDS_MAX_VM_CHANNELS) {
+       if (lcore_id >= RTE_MAX_LCORE) {
                RTE_LOG(ERR, POWER, "Core(%u) is out of range 0...%d\n",
-                               lcore_id, CHANNEL_CMDS_MAX_VM_CHANNELS-1);
+                               lcore_id, RTE_MAX_LCORE-1);
                return -1;
        }
        pkt[lcore_id].command = CPU_POWER;
@@ -68,9 +68,9 @@ send_msg(unsigned int lcore_id, uint32_t scale_direction)
 {
        int ret;
 
-       if (lcore_id >= CHANNEL_CMDS_MAX_VM_CHANNELS) {
+       if (lcore_id >= RTE_MAX_LCORE) {
                RTE_LOG(ERR, POWER, "Core(%u) is out of range 0...%d\n",
-                               lcore_id, CHANNEL_CMDS_MAX_VM_CHANNELS-1);
+                               lcore_id, RTE_MAX_LCORE-1);
                return -1;
        }
        pkt[lcore_id].unit = scale_direction;