[v2,1/1] ml/cnxk: fix multiple coverity issues
Checks
Commit Message
Added checks for null pointers. Removed logically dead code.
Fix division or modulo by zero. Fix evaluation order violation
issues. Fix potential memory leak in xstats function.
Coverity issue: 383658 383659, 383664, 383665
Fixes: 817e3e55bb18 ("ml/cnxk: support simulator environment")
Fixes: da3325131d71 ("ml/cnxk: find OCM mask and page slots for a model")
Fixes: b7d0650ebce0 ("ml/cnxk: reserve and free OCM pages")
Fixes: 515e3608c741 ("ml/cnxk: configure OCM page size")
Fixes: 4ff4ab8e1a20 ("ml/cnxk: support extended statistics")
Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
v2:
* additional null pointer checks
* xstats memory leak fix
drivers/ml/cnxk/cn10k_ml_dev.c | 4 ++--
drivers/ml/cnxk/cn10k_ml_ocm.c | 39 ++++++++++++----------------------
drivers/ml/cnxk/cn10k_ml_ops.c | 17 +++++++++++++--
3 files changed, 31 insertions(+), 29 deletions(-)
Comments
16/03/2023 10:33, Srikanth Yalavarthi:
> Added checks for null pointers. Removed logically dead code.
> Fix division or modulo by zero. Fix evaluation order violation
> issues. Fix potential memory leak in xstats function.
I think it would be better to split this patch a little,
so we can easily see what are the reasons for the changes.
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: 16 March 2023 22:30
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Cc: Prince Takkar <ptakkar@marvell.com>; Shivah Shankar Shankar Narayan
> Rao <sshankarnara@marvell.com>; dev@dpdk.org; Anup Prabhu
> <aprabhu@marvell.com>
> Subject: [EXT] Re: [PATCH v2 1/1] ml/cnxk: fix multiple coverity issues
>
> External Email
>
> ----------------------------------------------------------------------
> 16/03/2023 10:33, Srikanth Yalavarthi:
> > Added checks for null pointers. Removed logically dead code.
> > Fix division or modulo by zero. Fix evaluation order violation issues.
> > Fix potential memory leak in xstats function.
>
> I think it would be better to split this patch a little, so we can easily see what
> are the reasons for the changes.
Separate patch for each Coverity issue?
>
>
16/03/2023 18:02, Srikanth Yalavarthi:
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: 16 March 2023 22:30
> > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > Cc: Prince Takkar <ptakkar@marvell.com>; Shivah Shankar Shankar Narayan
> > Rao <sshankarnara@marvell.com>; dev@dpdk.org; Anup Prabhu
> > <aprabhu@marvell.com>
> > Subject: [EXT] Re: [PATCH v2 1/1] ml/cnxk: fix multiple coverity issues
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > 16/03/2023 10:33, Srikanth Yalavarthi:
> > > Added checks for null pointers. Removed logically dead code.
> > > Fix division or modulo by zero. Fix evaluation order violation issues.
> > > Fix potential memory leak in xstats function.
> >
> > I think it would be better to split this patch a little, so we can easily see what
> > are the reasons for the changes.
>
> Separate patch for each Coverity issue?
Maybe yes.
Or try to group similar causes.
@@ -779,8 +779,8 @@ cn10k_ml_fw_load(struct cn10k_ml_dev *mldev)
if (roc_env_is_emulator() || roc_env_is_hw()) {
/* Read firmware image to a buffer */
ret = rte_firmware_read(fw->path, &fw_buffer, &fw_size);
- if (ret < 0) {
- plt_err("Can't read firmware data: %s\n", fw->path);
+ if ((ret < 0) || (fw_buffer == NULL)) {
+ plt_err("Unable to read firmware data: %s\n", fw->path);
return ret;
}
@@ -224,7 +224,6 @@ cn10k_ml_ocm_tilemask_find(struct rte_ml_dev *dev, uint8_t num_tiles, uint16_t w
uint16_t scratch_page_start;
int used_last_wb_page_max;
uint16_t scratch_page_end;
- uint8_t search_start_tile;
uint8_t search_end_tile;
uint8_t *local_ocm_mask;
int wb_page_start_curr;
@@ -235,7 +234,6 @@ cn10k_ml_ocm_tilemask_find(struct rte_ml_dev *dev, uint8_t num_tiles, uint16_t w
uint16_t word_id;
uint8_t tile_idx;
int max_slot_sz;
- int start_tile;
int page_id;
mldev = dev->data->dev_private;
@@ -250,28 +248,18 @@ cn10k_ml_ocm_tilemask_find(struct rte_ml_dev *dev, uint8_t num_tiles, uint16_t w
wb_page_start = -1;
used_scratch_pages_max = 0;
used_last_wb_page_max = -1;
- start_tile = -1;
max_slot_sz_curr = 0;
max_slot_sz = 0;
tile_idx = 0;
+ tile_start = 0;
+ search_end_tile = ocm->num_tiles - num_tiles;
- if ((start_tile != -1) && (start_tile % num_tiles != 0)) {
- plt_err("Invalid start_tile, %d", start_tile);
+ local_ocm_mask = rte_zmalloc("local_ocm_mask", mldev->ocm.mask_words, RTE_CACHE_LINE_SIZE);
+ if (local_ocm_mask == NULL) {
+ plt_err("Unable to allocate memory for local_ocm_mask");
return -1;
}
- if (start_tile < 0) {
- search_start_tile = 0;
- search_end_tile = ocm->num_tiles - num_tiles;
- } else {
- search_start_tile = start_tile;
- search_end_tile = start_tile;
- }
-
- /* nibbles + prefix '0x' */
- local_ocm_mask = rte_zmalloc("local_ocm_mask", mldev->ocm.mask_words, RTE_CACHE_LINE_SIZE);
-
- tile_start = search_start_tile;
start_search:
used_scratch_pages_max = 0;
used_last_wb_page_max = -1;
@@ -423,10 +411,8 @@ cn10k_ml_ocm_free_pages(struct rte_ml_dev *dev, uint16_t model_id)
wb_page_end = wb_page_start + model->model_mem_map.wb_pages - 1;
for (tile_id = model->addr.tile_start; tile_id <= model->addr.tile_end; tile_id++) {
for (page_id = wb_page_start; page_id <= wb_page_end; page_id++) {
- ocm->tile_ocm_info[tile_id].ocm_mask[page_id / OCM_MAP_WORD_SIZE] =
- CLEAR_BIT(ocm->tile_ocm_info[tile_id]
- .ocm_mask[page_id / OCM_MAP_WORD_SIZE],
- page_id % OCM_MAP_WORD_SIZE);
+ CLEAR_BIT(ocm->tile_ocm_info[tile_id].ocm_mask[page_id / OCM_MAP_WORD_SIZE],
+ page_id % OCM_MAP_WORD_SIZE);
}
/* Update last_wb_page size */
@@ -452,10 +438,9 @@ cn10k_ml_ocm_free_pages(struct rte_ml_dev *dev, uint16_t model_id)
prev_start = ocm->num_pages - ocm->tile_ocm_info[tile_id].scratch_pages;
curr_start = ocm->num_pages - scratch_resize_pages;
for (page_id = prev_start; page_id < curr_start; page_id++) {
- ocm->tile_ocm_info[tile_id].ocm_mask[page_id / OCM_MAP_WORD_SIZE] =
- CLEAR_BIT(ocm->tile_ocm_info[tile_id]
- .ocm_mask[page_id / OCM_MAP_WORD_SIZE],
- page_id % OCM_MAP_WORD_SIZE);
+ CLEAR_BIT(ocm->tile_ocm_info[tile_id]
+ .ocm_mask[page_id / OCM_MAP_WORD_SIZE],
+ page_id % OCM_MAP_WORD_SIZE);
}
ocm->tile_ocm_info[tile_id].scratch_pages = scratch_resize_pages;
}
@@ -497,6 +482,10 @@ cn10k_ml_ocm_print(struct rte_ml_dev *dev, FILE *fp)
/* nibbles + prefix '0x' */
str = rte_zmalloc("ocm_mask_str", mldev->ocm.num_pages / 4 + 2, RTE_CACHE_LINE_SIZE);
+ if (str == NULL) {
+ plt_err("Unable to allocate memory for ocm_mask_str");
+ return;
+ }
fprintf(fp, "OCM State:\n");
for (tile_id = 0; tile_id < ocm->num_tiles; tile_id++) {
@@ -444,7 +444,8 @@ cn10k_ml_prep_fp_job_descriptor(struct rte_ml_dev *dev, struct cn10k_ml_req *req
count += model->burst_stats[qp_id].dequeued_count - \
model->burst_stats[qp_id].str##_reset_count; \
} \
- value = value / count; \
+ if (count != 0) \
+ value = value / count; \
} while (0)
#define ML_MIN_FOREACH_QP(dev, model, qp_id, str, value, count) \
@@ -788,6 +789,11 @@ cn10k_ml_dev_configure(struct rte_ml_dev *dev, const struct rte_ml_dev_config *c
/* Allocate memory for ocm_mask */
ocm->ocm_mask =
rte_zmalloc("ocm_mask", ocm->mask_words * ocm->num_tiles, RTE_CACHE_LINE_SIZE);
+ if (ocm->ocm_mask == NULL) {
+ plt_err("Unable to allocate memory for OCM mask");
+ ret = -ENOMEM;
+ goto error;
+ }
for (tile_id = 0; tile_id < ocm->num_tiles; tile_id++) {
ocm->tile_ocm_info[tile_id].ocm_mask = ocm->ocm_mask + tile_id * ocm->mask_words;
@@ -1106,6 +1112,11 @@ cn10k_ml_dev_xstats_by_name_get(struct rte_ml_dev *dev, const char *name, uint16
num_xstats = PLT_DIM(cn10k_ml_model_xstats_table) * mldev->nb_models_loaded;
xstats_map = rte_zmalloc("cn10k_ml_xstats_map",
sizeof(struct rte_ml_dev_xstats_map) * num_xstats, 0);
+ if (xstats_map == NULL) {
+ plt_err("Unable to allocate memory for cn10k_ml_xstats_map");
+ return -ENOMEM;
+ }
+
cn10k_ml_dev_xstats_names_get(dev, xstats_map, num_xstats);
cn10k_ml_dev_info_get(dev, &dev_info);
@@ -1117,8 +1128,10 @@ cn10k_ml_dev_xstats_by_name_get(struct rte_ml_dev *dev, const char *name, uint16
}
}
- if (id == PLT_DIM(cn10k_ml_model_xstats_table) * dev_info.max_models)
+ if (id == PLT_DIM(cn10k_ml_model_xstats_table) * dev_info.max_models) {
+ rte_free(xstats_map);
return -EINVAL;
+ }
model_id = id / PLT_DIM(cn10k_ml_model_xstats_table);
type = id % PLT_DIM(cn10k_ml_model_xstats_table);