Skip to content

Commit

Permalink
Fix for static analysis issues
Browse files Browse the repository at this point in the history 
Changes include:
- Initialize the uninitialized varibles.
- Fix for Memory leak by releasing the allocated memory.
- Add check for self assign issue
- Typecast the variables to appropriate type.
- Fix for unused return values by adding a check for returned value.
- Fix for unreachable code by removing unnecessary checks.
- Fix for buffer overflow issue by limiting the size of dst buffer as
minimum of the two(src and dst buffer).

Tracked-On: OAM-111171
Signed-off-by: Singh, Sapna1 <[email protected]>
  • Loading branch information
@Sapna1-singh @sysopenci
Sapna1-singh authored and sysopenci committed Jul 20, 2023
1 parent bdf3ad6 commit 0b1339c
Showing 1 changed file with 255 additions and 0 deletions.
255 changes: 255 additions & 0 deletions ...dor/intel/external/project-celadon/mediasdk-omx/0008-Fix-for-static-analysis-issues.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,255 @@
From 5cee3a94cc8c5d85b2e5ab4d39b1aa3b81386e1e Mon Sep 17 00:00:00 2001
From: "Singh, Sapna1" <[email protected]>
Date: Fri, 14 Jul 2023 15:55:31 +0530
Subject: [PATCH] [PATCH] Fix for static analysis issues

Changes include:
- Initialize the uninitialized varibles.
- Fix for Memory leak by releasing the allocated memory.
- Add check for self assign issue
- Typecast the variables to appropriate type.
- Fix for unused return values by adding a check for returned value.
- Fix for unreachable code by removing unnecessary checks.
- Fix for buffer overflow issue by limiting the size of dst buffer as
minimum of the two(src and dst buffer).

Tracked-On: OAM-111171
Signed-off-by: Singh, Sapna1 <[email protected]>
---
omx_components/src/mfx_omx_vdec_component.cpp | 9 ++---
omx_components/src/mfx_omx_venc_component.cpp | 23 ++++++++-----
omx_components/src/mfx_omx_vpp_wrapp.cpp | 1 +
omx_core/src/mfx_omx_core.cpp | 14 ++++++--
omx_utils/include/mfx_omx_types.h | 33 ++++++++++---------
omx_utils/src/mfx_omx_utils.cpp | 6 ++--
omx_utils/src/mfx_omx_vaapi_allocator.cpp | 2 +-
7 files changed, 53 insertions(+), 35 deletions(-)

diff --git a/omx_components/src/mfx_omx_vdec_component.cpp b/omx_components/src/mfx_omx_vdec_component.cpp
index f3e02ec..9586385 100644
--- a/omx_components/src/mfx_omx_vdec_component.cpp
+++ b/omx_components/src/mfx_omx_vdec_component.cpp
@@ -170,7 +170,7 @@ OMX_ERRORTYPE MfxOmxVdecComponent::Init(void)
Reset();

mfxConfig cfg[2];
- mfxVariant cfgVal[2];
+ mfxVariant cfgVal[2] = {};

m_Loader = MFXLoad();
if (nullptr == m_Loader) {
@@ -1513,8 +1513,8 @@ OMX_ERRORTYPE MfxOmxVdecComponent::GetConfig(
{
omx_res = OMX_ErrorUnsupportedSetting;
}
-
- omx_res = OMX_ErrorNone;
+ else
+ omx_res = OMX_ErrorNone;
break;
}
#ifdef HEVC10HDR_SUPPORT
@@ -2313,7 +2313,8 @@ void MfxOmxVdecComponent::MainThread(void)
if (pWorkSurface)
{
mfx_sts = m_pSurfaces->QueueBufferForSending(pWorkSurface, NULL);
-
+ if (MFX_ERR_NONE == mfx_sts)
+ MFX_OMX_AUTO_TRACE("Queueing Buffers for Sending");
m_bEosHandlingFinished = true;

m_pAsyncSemaphore->Post();
diff --git a/omx_components/src/mfx_omx_venc_component.cpp b/omx_components/src/mfx_omx_venc_component.cpp
index d915c16..0c6f423 100755
--- a/omx_components/src/mfx_omx_venc_component.cpp
+++ b/omx_components/src/mfx_omx_venc_component.cpp
@@ -159,7 +159,7 @@ OMX_ERRORTYPE MfxOmxVencComponent::Init(void)
// prepare Media SDK
#ifdef USE_ONEVPL
mfxConfig cfg[2];
- mfxVariant cfgVal[2];
+ mfxVariant cfgVal[2] = {};

m_Loader = MFXLoad();
if (nullptr == m_Loader) {
@@ -1312,6 +1312,8 @@ OMX_ERRORTYPE MfxOmxVencComponent::DealWithBuffer(
}
*ppBufferHdr = pBufferHeader;
}
+ else
+ MFX_OMX_FREE(pBufferHeader);
}
MFX_OMX_AUTO_TRACE_U32(omx_res);
return omx_res;
@@ -1787,9 +1789,9 @@ OMX_ERRORTYPE MfxOmxVencComponent::SetParameter(
MFX_OMX_AUTO_TRACE_I32(pParam->bStoreMetaData);
{
#ifdef USE_ONEVPL
- m_MfxVideoParams.IOPattern &= !(MFX_IOPATTERN_IN_VIDEO_MEMORY | MFX_IOPATTERN_IN_SYSTEM_MEMORY);
+ m_MfxVideoParams.IOPattern &= ~(MFX_IOPATTERN_IN_VIDEO_MEMORY | MFX_IOPATTERN_IN_SYSTEM_MEMORY);
#else
- m_MfxVideoParams.IOPattern &= !(MFX_IOPATTERN_IN_VIDEO_MEMORY | MFX_IOPATTERN_IN_SYSTEM_MEMORY | MFX_IOPATTERN_IN_OPAQUE_MEMORY);
+ m_MfxVideoParams.IOPattern &= ~(MFX_IOPATTERN_IN_VIDEO_MEMORY | MFX_IOPATTERN_IN_SYSTEM_MEMORY | MFX_IOPATTERN_IN_OPAQUE_MEMORY);
#endif
m_MfxVideoParams.IOPattern |= pParam->bStoreMetaData ? MFX_IOPATTERN_IN_VIDEO_MEMORY : MFX_IOPATTERN_IN_SYSTEM_MEMORY;

@@ -2029,8 +2031,8 @@ OMX_ERRORTYPE MfxOmxVencComponent::GetBlackFrame(OMX_VIDEO_INTEL_REQUEST_BALCK_F

if (MFX_ERR_NONE == mfx_res)
{
- memset(data.Y, 0x0, data.Pitch * m_MfxVideoParams.mfx.FrameInfo.Height);
- memset(data.U, 0x80, data.Pitch * m_MfxVideoParams.mfx.FrameInfo.Height / 2);
+ memset(data.Y, 0x0, (mfxU64)data.Pitch * m_MfxVideoParams.mfx.FrameInfo.Height);
+ memset(data.U, 0x80, (mfxU64)data.Pitch * m_MfxVideoParams.mfx.FrameInfo.Height / 2);
mfx_res = gralloc->UnlockFrame(handle, &data);

m_pSurfaces->SetBlackFrame(handle);
@@ -2536,6 +2538,8 @@ void MfxOmxVencComponent::MainThread(void)
{
MFX_OMX_AUTO_TRACE("handling EOS");
mfx_sts = ProcessEOS();
+ if (MFX_ERR_NONE == mfx_sts)
+ MFX_OMX_LOG_INFO_IF(g_OmxLogLevel, "MainThread : No Error in handling EOS");
// If error happened at the end of stream we skip this
}
}
@@ -2624,9 +2628,10 @@ mfxStatus MfxOmxVencComponent::ProcessEOS(void)

m_pAsyncSemaphore->Post();
}
- else m_bCanNotProcess = true;
-
- mfx_res = MFX_ERR_NONE;
+ else {
+ m_bCanNotProcess = true;
+ mfx_res = MFX_ERR_NONE;
+ }
}
MFX_OMX_AUTO_TRACE_I32(m_bEosHandlingStarted);
MFX_OMX_AUTO_TRACE_I32(m_bEosHandlingFinished);
@@ -3229,7 +3234,7 @@ void MfxOmxVencComponent::SkipFrame(void)
m_lCurTargetBitrate = m_MfxVideoParams.mfx.TargetKbps * 1000;
MFX_OMX_LOG_INFO_IF(g_OmxLogLevel, "[SkipFrameLog] ###Target bitrate change.###");
}
- mfxI64 avgFrameSize = m_MfxVideoParams.mfx.TargetKbps * 1000 * m_MfxVideoParams.mfx.FrameInfo.FrameRateExtD / m_MfxVideoParams.mfx.FrameInfo.FrameRateExtN;
+ mfxI64 avgFrameSize = (mfxI64)(m_MfxVideoParams.mfx.TargetKbps * 1000) * m_MfxVideoParams.mfx.FrameInfo.FrameRateExtD / m_MfxVideoParams.mfx.FrameInfo.FrameRateExtN;
m_lBufferFullness += pAddBufInfo2->sBitstream.DataLength * 8 - avgFrameSize;

mfxI64 overFlowBound = 0.8 * m_MfxVideoParams.mfx.TargetKbps * 1000;
diff --git a/omx_components/src/mfx_omx_vpp_wrapp.cpp b/omx_components/src/mfx_omx_vpp_wrapp.cpp
index 4ebe9e1..68ee14b 100644
--- a/omx_components/src/mfx_omx_vpp_wrapp.cpp
+++ b/omx_components/src/mfx_omx_vpp_wrapp.cpp
@@ -46,6 +46,7 @@ MfxOmxVppWrapp::MfxOmxVppWrapp(void):
MFX_OMX_ZERO_MEMORY(m_vppParam);
MFX_OMX_ZERO_MEMORY(m_allocator);
MFX_OMX_ZERO_MEMORY(m_responses);
+ MFX_OMX_ZERO_MEMORY(m_vppSrf);
}

/*------------------------------------------------------------------------------*/
diff --git a/omx_core/src/mfx_omx_core.cpp b/omx_core/src/mfx_omx_core.cpp
index 8de5c43..0d21f41 100644
--- a/omx_core/src/mfx_omx_core.cpp
+++ b/omx_core/src/mfx_omx_core.cpp
@@ -323,9 +323,17 @@ static OMX_ERRORTYPE mfx_omx_read_config_file(void)
if (components)
{
g_ComponentsRegistry = components;
-
- strcpy(g_ComponentsRegistry[g_ComponentsRegistryNum].m_component_name, name);
- strcpy(g_ComponentsRegistry[g_ComponentsRegistryNum].m_component_so, value);
+ size_t name_max_len =
+ sizeof(g_ComponentsRegistry[g_ComponentsRegistryNum].m_component_name) - 1;
+ size_t so_max_len =
+ sizeof(g_ComponentsRegistry[g_ComponentsRegistryNum].m_component_so) - 1;
+
+ g_ComponentsRegistry[g_ComponentsRegistryNum].m_component_name[name_max_len] = '\0';
+ g_ComponentsRegistry[g_ComponentsRegistryNum].m_component_so[so_max_len] = '\0';
+ strncpy(g_ComponentsRegistry[g_ComponentsRegistryNum].m_component_name, name,
+ std::min(name_max_len, strlen(name)));
+ strncpy(g_ComponentsRegistry[g_ComponentsRegistryNum].m_component_so, value,
+ std::min(so_max_len, strlen(value)));
g_ComponentsRegistry[g_ComponentsRegistryNum].m_component_flags =
(str_flags)? strtol(str_flags, NULL, 16): (OMX_U32)MFX_OMX_COMPONENT_FLAGS_NONE;

diff --git a/omx_utils/include/mfx_omx_types.h b/omx_utils/include/mfx_omx_types.h
index 4d1b867..18291dc 100644
--- a/omx_utils/include/mfx_omx_types.h
+++ b/omx_utils/include/mfx_omx_types.h
@@ -219,22 +219,23 @@ struct MfxOmxParamsWrapper: public T
}
MfxOmxParamsWrapper& operator=(const MfxOmxParamsWrapper& ref)
{
- T* dst = this;
- const T* src = &ref;
-
- *dst = *src;
- payload = ref.payload;
- if (!N) return *this;
-
- MFX_OMX_ZERO_MEMORY(ext_buf_ptrs);
- std::copy(std::begin(ref.ext_buf), std::end(ref.ext_buf), std::begin(ext_buf));
- std::copy(std::begin(ref.ext_buf_idxmap), std::end(ref.ext_buf_idxmap), std::begin(ext_buf_idxmap));
-
- this->ExtParam = ext_buf_ptrs;
- for (size_t i = 0; i < N; ++i)
- {
- ext_buf_ptrs[i] = (mfxExtBuffer*)&ext_buf[i];
- }
+ if (this != &ref) {
+ T* dst = this;
+ const T* src = &ref;
+
+ *dst = *src;
+ payload = ref.payload;
+ if (!N) return *this;
+
+ MFX_OMX_ZERO_MEMORY(ext_buf_ptrs);
+ std::copy(std::begin(ref.ext_buf), std::end(ref.ext_buf), std::begin(ext_buf));
+ std::copy(std::begin(ref.ext_buf_idxmap), std::end(ref.ext_buf_idxmap), std::begin(ext_buf_idxmap));
+ this->ExtParam = ext_buf_ptrs;
+ for (size_t i = 0; i < N; ++i)
+ {
+ ext_buf_ptrs[i] = (mfxExtBuffer*)&ext_buf[i];
+ }
+ }
return *this;
}
MfxOmxParamsWrapper(const T& ref)
diff --git a/omx_utils/src/mfx_omx_utils.cpp b/omx_utils/src/mfx_omx_utils.cpp
index 87a525c..b3e1d47 100644
--- a/omx_utils/src/mfx_omx_utils.cpp
+++ b/omx_utils/src/mfx_omx_utils.cpp
@@ -480,11 +480,13 @@ mfxU32 GetBufferSize(mfxVideoParam const & par)
maxKbps * DEFAULT_CPB_IN_SECONDS);

bufferSize = !IsHRDBasedBRCMethod(par.mfx.RateControlMethod)
- ? mfxU32(MFX_OMX_MIN(UINT_MAX, par.mfx.FrameInfo.Width * par.mfx.FrameInfo.Height * 3 / 2))
+ //? mfxU32(MFX_OMX_MIN(UINT_MAX, par.mfx.FrameInfo.Width * par.mfx.FrameInfo.Height * 3 / 2))
+ ? mfxU32(par.mfx.FrameInfo.Width * par.mfx.FrameInfo.Height * 3 / 2)
: bufferSizeInBits / 8;
}
else
- bufferSize = mfxU32(MFX_OMX_MIN(UINT_MAX, par.mfx.FrameInfo.Width * par.mfx.FrameInfo.Height * 3 / 2));
+ //bufferSize = mfxU32(MFX_OMX_MIN(UINT_MAX, par.mfx.FrameInfo.Width * par.mfx.FrameInfo.Height * 3 / 2));
+ bufferSize = mfxU32(par.mfx.FrameInfo.Width * par.mfx.FrameInfo.Height * 3 / 2);

return bufferSize;
}
diff --git a/omx_utils/src/mfx_omx_vaapi_allocator.cpp b/omx_utils/src/mfx_omx_vaapi_allocator.cpp
index 5fae460..5f89522 100644
--- a/omx_utils/src/mfx_omx_vaapi_allocator.cpp
+++ b/omx_utils/src/mfx_omx_vaapi_allocator.cpp
@@ -277,7 +277,7 @@ mfxStatus MfxOmxVaapiFrameAllocator::RegisterSurface(VASurfaceID surface, mfxMem
pmid->m_bUseBufferDirectly = bUseBufferDirectly;
out_mid = pmid;
m_extMIDs.push_back(pmid);
- if (out_mid)
+ if (out_mid != NULL)
*mid = out_mid;
else
mfx_res = MFX_ERR_UNKNOWN;
--
2.40.0

0 comments on commit 0b1339c

Please sign in to comment.