Skip to content

Commit

Permalink
Fix: fix and refactor st41 possible errors (#972)
Browse files Browse the repository at this point in the history
* Fix possible null pointer reference in app_tx_fmd_init

* Fix: coveriy out of band issue

* Increate quality of tx_fastmetadata_session_build_packet code

* coverity fix:  fixing less-than-zero comparison

* add st41 context app multithread access error

* Add error message when the st41 functionality of RxTx app detects
multithread access to the contex
app resource, which should not be shared.

Fixes: c43152d

Co-developed-by: Aleksandr Ivanov <aleksandr.ivanov@intel.com>
Co-developed-by: Kolelis, Szymon <szymon.kolelis@intel.com>
  • Loading branch information
DawidWesierski4 authored Sep 17, 2024
1 parent ef90ace commit 664344a
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 13 deletions.
6 changes: 4 additions & 2 deletions app/src/parse_json.c
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,7 @@ static int st_json_parse_tx_fmd(int idx, json_object* fmd_obj,
st_json_object_object_get(fmd_obj, "fastmetadata_data_item_type");
if (fmd_dit_obj) {
uint32_t fmd_dit = json_object_get_int(fmd_dit_obj);
if (fmd_dit < 0 || fmd_dit > 0x3fffff) {
if (fmd_dit > 0x3fffff) {
err("%s, invalid fastmetadata_data_item_type 0x%x\n", __func__, fmd_dit);
return -ST_JSON_NOT_VALID;
}
Expand All @@ -1269,8 +1269,10 @@ static int st_json_parse_tx_fmd(int idx, json_object* fmd_obj,
/* parse fmd data item K-bit */
json_object* fmd_k_bit_obj = st_json_object_object_get(fmd_obj, "fastmetadata_k_bit");
if (fmd_k_bit_obj) {
/* assign to uint and check if the value more then 1
* (the value should be in range of [0,1]) */
uint8_t fmd_k_bit = json_object_get_int(fmd_k_bit_obj);
if (fmd_k_bit < 0 || fmd_k_bit > 1) {
if (fmd_k_bit > 1) {
err("%s, invalid fastmetadata_k_bit 0x%x\n", __func__, fmd_k_bit);
return -ST_JSON_NOT_VALID;
}
Expand Down
22 changes: 15 additions & 7 deletions app/src/tx_fastmetadata_app.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,21 +94,29 @@ static void* app_tx_fmd_frame_thread(void* arg) {
info("%s(%d), start\n", __func__, idx);
while (!s->st41_app_thread_stop) {
st_pthread_mutex_lock(&s->st41_wake_mutex);
producer_idx = s->framebuff_producer_idx;
framebuff = &s->framebuffs[producer_idx];
if (ST_TX_FRAME_FREE != framebuff->stat) {
/* not in free */
if (!s->st41_app_thread_stop)
framebuff = &s->framebuffs[s->framebuff_producer_idx];

if (framebuff->stat != ST_TX_FRAME_FREE) {
if (!s->st41_app_thread_stop) {
st_pthread_cond_wait(&s->st41_wake_cond, &s->st41_wake_mutex);
}
st_pthread_mutex_unlock(&s->st41_wake_mutex);
continue;
}

producer_idx = s->framebuff_producer_idx;
st_pthread_mutex_unlock(&s->st41_wake_mutex);

struct st41_frame* frame_addr = st41_tx_get_framebuffer(s->handle, producer_idx);
app_tx_fmd_build_frame(s, frame_addr);

st_pthread_mutex_lock(&s->st41_wake_mutex);
if (producer_idx != s->framebuff_producer_idx) {
err("%s(%d), Apps can't share the context app structure\n", __func__, idx);
st_pthread_mutex_unlock(&s->st41_wake_mutex);
return NULL;
}

framebuff->size = sizeof(*frame_addr);
framebuff->stat = ST_TX_FRAME_READY;
/* point to next */
Expand Down Expand Up @@ -440,8 +448,8 @@ static int app_tx_fmd_init(struct st_app_context* ctx,
ops.notify_rtp_done = app_tx_fmd_rtp_done;
ops.framebuff_cnt = s->framebuff_cnt;
ops.fps = fmd ? fmd->info.fmd_fps : ST_FPS_P59_94;
ops.fmd_dit = fmd->info.fmd_dit;
ops.fmd_k_bit = fmd->info.fmd_k_bit;
ops.fmd_dit = fmd ? fmd->info.fmd_dit : 0;
ops.fmd_k_bit = fmd ? fmd->info.fmd_k_bit : 0;
s->st41_pcap_input = false;
ops.type = fmd ? fmd->info.type : ST41_TYPE_FRAME_LEVEL;
ops.interlaced = fmd ? fmd->info.interlaced : false;
Expand Down
18 changes: 14 additions & 4 deletions lib/src/st2110/st_tx_fastmetadata_session.c
Original file line number Diff line number Diff line change
Expand Up @@ -389,15 +389,20 @@ static int tx_fastmetadata_session_update_redundant(

static void tx_fastmetadata_session_build_packet(
struct st_tx_fastmetadata_session_impl* s, struct rte_mbuf* pkt) {
struct mt_udp_hdr* hdr;
struct st41_fmd_hdr* hdr;
struct rte_ipv4_hdr* ipv4;
struct rte_udp_hdr* udp;
struct st41_rtp_hdr* rtp;

hdr = rte_pktmbuf_mtod(pkt, struct mt_udp_hdr*);
if (rte_pktmbuf_data_len(pkt) < sizeof(*hdr)) {
err("%s: packet is less than fmd hdr size", __func__);
return;
}

hdr = rte_pktmbuf_mtod(pkt, struct st41_fmd_hdr*);
ipv4 = &hdr->ipv4;
udp = &hdr->udp;
rtp = (struct st41_rtp_hdr*)&udp[1];
rtp = &hdr->rtp;

/* copy the hdr: eth, ip, udp */
rte_memcpy(&hdr->eth, &s->hdr[MTL_SESSION_PORT_P].eth, sizeof(hdr->eth));
Expand All @@ -417,7 +422,6 @@ static void tx_fastmetadata_session_build_packet(
rtp->base.tmstamp = htonl(s->pacing.rtp_time_stamp);

/* Set place for payload just behind rtp header */
uint8_t* payload = (uint8_t*)&rtp[1];
struct st_frame_trans* frame_info = &s->st41_frames[s->st41_frame_idx];
uint32_t offset = s->st41_pkt_idx * s->max_pkt_len;
void* src_addr = frame_info->addr + offset;
Expand All @@ -426,6 +430,12 @@ static void tx_fastmetadata_session_build_packet(
uint16_t data_item_length =
(data_item_length_bytes + 3) / 4; /* expressed in number of 4-byte words */

if (rte_pktmbuf_data_len(pkt) < sizeof(*hdr) + data_item_length) {
err("%s: packet doesn't contain RTP payload", __func__);
return;
}

uint8_t* payload = (uint8_t*)(rtp + 1);
if (!(data_item_length_bytes > s->max_pkt_len)) {
int offset = 0;
for (int i = 0; i < data_item_length_bytes; i++) {
Expand Down

0 comments on commit 664344a

Please sign in to comment.