Skip to content

Commit 7de03cb

Browse files
haoxian2pcolberg
authored andcommitted
acl_platform: fix coverity out-of-bounds read and write (OVERRUN)
Fix potential out of bounds read from and write to shipped_board_def.
1 parent f8e3a83 commit 7de03cb

File tree

3 files changed

+51
-55
lines changed

3 files changed

+51
-55
lines changed

include/acl_shipped_board_cfgs.h

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
// Copyright (C) 2012-2021 Intel Corporation
22
// SPDX-License-Identifier: BSD-3-Clause
3+
#include <array>
34

4-
/* Format is board name, then sys description, then terminated by two NULLs */
5-
static const char *acl_shipped_board_cfgs[2 * (ACL_MAX_DEVICE + 1)] = {
6-
"a10gx",
7-
"23 16 sample40byterandomhash000000000000000000 a10gx 0 1 9 DDR 2 1 2 0 "
8-
"2147483648 0 - 0 0 0 0 0 ",
9-
"a10_ref_small",
10-
"23 16 sample40byterandomhash000000000000000000 a10_ref_small 0 1 9 DDR 2 "
11-
"1 2 0 134217728 0 - 0 0 0 0 0 ",
12-
"s10gx",
13-
"23 16 sample40byterandomhash000000000000000000 s10gx_ea 0 1 9 DDR 2 1 2 0 "
14-
"2147483648 0 - 0 0 0 0 0 ",
15-
0,
16-
0};
5+
/* Format is std::array of struct containing board name and sys description */
6+
struct shipped_board_cfg {
7+
const char *name;
8+
const char *cfg;
9+
};
10+
11+
static const std::array<shipped_board_cfg, 3> acl_shipped_board_cfgs = {{
12+
{"a10gx", "23 16 sample40byterandomhash000000000000000000 a10gx 0 1 9 "
13+
"DDR 2 1 2 0 2147483648 0 - 0 0 0 0 0 "},
14+
{"a10_ref_small",
15+
"23 16 sample40byterandomhash000000000000000000 a10_ref_small 0 1 9 "
16+
"DDR 2 1 2 0 134217728 0 - 0 0 0 0 0 "},
17+
{"s10gx", "23 16 sample40byterandomhash000000000000000000 s10gx_ea 0 "
18+
"1 9 DDR 2 1 2 0 2147483648 0 - 0 0 0 0 0 "},
19+
}};

src/acl_kernel_if.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -725,8 +725,8 @@ int acl_kernel_if_init(acl_kernel_if *kern, acl_bsp_io bsp_io,
725725
if (use_offline_only == ACL_CONTEXT_MPSIM) {
726726
std::string err_msg;
727727
auto parse_result = acl_load_device_def_from_str(
728-
std::string(acl_shipped_board_cfgs[1]),
729-
sysdef->device[0].autodiscovery_def, err_msg);
728+
acl_shipped_board_cfgs[0].cfg, sysdef->device[0].autodiscovery_def,
729+
err_msg);
730730
// Fill in definition for all device global memory
731731
// Simulator does not have any global memory interface information until the
732732
// actual aocx is loaded. (Note this is only a problem for simulator not

src/acl_platform.cpp

Lines changed: 33 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33

44
// System headers.
55
#include <assert.h>
6+
#include <optional>
67
#include <stddef.h>
78
#include <stdio.h>
89
#include <string.h>
10+
#include <vector>
911

1012
#ifdef _WIN32
1113
#include <windows.h>
@@ -68,8 +70,7 @@ cl_platform_id acl_get_platform() { return &acl_platform; }
6870
// Static global copies of shipped board definitions.
6971
// Need either this storage, or a way to deep copy the accelerator
7072
// definitions.
71-
static acl_system_def_t shipped_board_def[ACL_MAX_DEVICE];
72-
static int shipped_board_def_valid[ACL_MAX_DEVICE + 1];
73+
static std::vector<std::optional<acl_system_def_t>> shipped_board_defs;
7374

7475
//////////////////////////////
7576
// Local functions.
@@ -569,8 +570,6 @@ static void l_show_devs(const char *prefix) {
569570
}
570571

571572
static void l_initialize_offline_devices(int offline_mode) {
572-
unsigned i = 0;
573-
574573
acl_platform.global_mem.range.begin = 0;
575574
acl_platform.global_mem.range.next = 0;
576575

@@ -579,38 +578,37 @@ static void l_initialize_offline_devices(int offline_mode) {
579578
//
580579
// See acl_shipped_board_cfgs.h which is generated via the
581580
// board_cfgs makefile target.
582-
// It's an array of strings where index 2k is board name, and 2k+1 is
583-
// the auto configuration string.
584-
// The array always terminates with two NULLs.
585-
for (i = 0; i < ACL_MAX_DEVICE + 1; i++)
586-
shipped_board_def_valid[i] = 0;
587-
588-
for (i = 0; i < ACL_MAX_DEVICE &&
589-
2 * i < sizeof(acl_shipped_board_cfgs) / sizeof(const char *);
590-
i++) {
581+
// It's an array of shipped_board_cfg struct where name is board name, and
582+
// board_cfgs is the auto configuration string.
583+
shipped_board_defs.clear();
584+
for (const auto &board_cfg : acl_shipped_board_cfgs) {
591585
// This is always different storage.
592586
// We need this because l_add_device will just pointer-copy the
593587
// acl_device_def_t.
594-
if (!acl_shipped_board_cfgs[2 * i] || !acl_shipped_board_cfgs[2 * i + 1])
595-
break; // No more entries
596-
const std::string board_cfg{acl_shipped_board_cfgs[2 * i + 1]};
588+
auto &board_def = shipped_board_defs.emplace_back();
589+
board_def.emplace();
597590
std::string err_msg;
598-
shipped_board_def_valid[i] = acl_load_device_def_from_str(
599-
board_cfg, shipped_board_def[i].device[0].autodiscovery_def, err_msg);
600-
if (shipped_board_def_valid[i])
601-
shipped_board_def[i].num_devices = 1;
591+
if (!acl_load_device_def_from_str(
592+
board_cfg.cfg, board_def->device[0].autodiscovery_def, err_msg)) {
593+
board_def.reset();
594+
continue;
595+
}
596+
board_def->num_devices = 1;
602597
}
603598

604599
if (offline_mode == ACL_CONTEXT_MPSIM) {
600+
auto &board_def = shipped_board_defs.emplace_back();
601+
board_def.emplace();
605602
std::string err_msg;
606-
shipped_board_def_valid[i] = acl_load_device_def_from_str(
607-
std::string(acl_shipped_board_cfgs[1]),
608-
shipped_board_def[i].device[0].autodiscovery_def, err_msg);
609-
if (shipped_board_def_valid[i])
610-
shipped_board_def[i].num_devices = 1;
611-
// Need to change the name here or before we send the string
612-
shipped_board_def[i].device[0].autodiscovery_def.name =
613-
ACL_MPSIM_DEVICE_NAME;
603+
if (!acl_load_device_def_from_str(acl_shipped_board_cfgs[0].cfg,
604+
board_def->device[0].autodiscovery_def,
605+
err_msg)) {
606+
board_def.reset();
607+
} else {
608+
board_def->num_devices = 1;
609+
// Need to change the name here or before we send the string
610+
board_def->device[0].autodiscovery_def.name = ACL_MPSIM_DEVICE_NAME;
611+
}
614612
}
615613

616614
if (!acl_platform.offline_device.empty()) {
@@ -624,27 +622,22 @@ static void l_initialize_offline_devices(int offline_mode) {
624622

625623
// If the user specified an offline device, then load it.
626624
// Search the shipped board defs for the device:
627-
for (i = 0;
628-
i < ACL_MAX_DEVICE &&
629-
2 * i < sizeof(acl_shipped_board_cfgs) / sizeof(const char *) + 1;
630-
i++) {
631-
int is_present = 0;
632-
if (!shipped_board_def_valid[i])
625+
for (const auto &board_def : shipped_board_defs) {
626+
if (!board_def.has_value())
633627
continue;
634628
if (acl_platform.offline_device !=
635-
shipped_board_def[i].device[0].autodiscovery_def.name)
629+
board_def->device[0].autodiscovery_def.name)
636630
continue;
637631

638-
is_present = (offline_mode == ACL_CONTEXT_MPSIM);
632+
const bool is_present = (offline_mode == ACL_CONTEXT_MPSIM);
639633
for (unsigned j = 0; j < board_count; j++) {
640634
// Bail if not present and we haven't been told to use absent devices
641-
if (!is_present &&
642-
acl_platform.offline_device !=
643-
shipped_board_def[i].device[0].autodiscovery_def.name)
635+
if (!is_present && acl_platform.offline_device !=
636+
board_def->device[0].autodiscovery_def.name)
644637
continue;
645638
// Add HW specific device definition
646639
acl_platform.device[device_index].def =
647-
shipped_board_def[i].device[0]; // Struct Copy
640+
board_def->device[0]; // Struct Copy
648641
acl_platform.device[device_index].present = is_present;
649642
device_index++;
650643
}

0 commit comments

Comments
 (0)