Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

RDK-55945 : sync codebase with stable2 #16

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

shivabhaskar
Copy link
Contributor

No description provided.

PriyaKathiravan and others added 13 commits February 11, 2025 13:45
Reason for change: To change the char* arguments to const char* in public apis
Test Procedure: check that the event markers are observed in the telemetry reports and no crashes observed.
Risks: Medium
Priority: P1
Signed-off-by: pkathi542 <PriyaDharshini_Kathiravan@comcast.com>

Change-Id: I283a5ac12d5f12870b9e47672082b915732f735a
(cherry picked from commit fc2409bd813446fbfb44d982c51a92b2c0368634)
…r to NULL

Reason for change:
set singleProfile->cachedReportList pointer to NULL
after calling Vector_Destroy()
Code cleanup based analysis of a crash report.
Test Procedure: Sanity test.
Risks: None.
Signed-off-by: mkamat <mkamat@libertyglobal.com>
Priority: P1

Change-Id: Id336535528c1dd423ccf63c18f3cebd91ad71770
(cherry picked from commit e47fe74b09c053998823285ff60a8e363e601f3e)
Reason for change:
Fix memory leaks related to the usage of cJSON_Parse and cJSONPrint.
Test Procedure:
Expected result - No memory leaks for cJSON_Parse and cJSONPrint..
A basic sanity testing of telemetry component is required for this change.
Risks: Low
Signed-off-by: Marcin Tomczyk <marcin.tomczyk@cognizant.com>
Priority: P1

Change-Id: Ib212598bcefd9556c99c150bb8b06b4fca601a35
(cherry picked from commit 9c2e49fc69735058cc0a2b7f00e8db2305b6af68)
Reason for change: T2 profiles should be persistent after reboot or restart
Test Procedure: Verify whether the previous profile is running in the device after t2 restart
Risks: Medium
Priority: P1
Signed-off-by: pkathi542 <PriyaDharshini_Kathiravan@comcast.com>

Change-Id: I4514078178416bb7e1d4f925acd51d3647bf3c88
Reason for change: Removed the mutex locks from collectandReportxconf thread to avoid the missing of telemetry uploads in device and elastic.
Test Procedure: Build and Verify whether the single profile report is generated every 15 minutes.
Risks: Medium
Priority: P1
Signed-off-by: pkathi542 <PriyaDharshini_Kathiravan@comcast.com>

Change-Id: I2398aa4576af95374c833104d208f13d58b12566
(cherry picked from commit ecee5004dea7b094ee4d0e82439110d56e06b13d)
Reason for change: Move the code to check the profile exixtence
  under the mutex protection plMutex to avoid inconsistencey
  and crashes
Test Procedure: Crash should not be observed when un registering
  the profile from the scheduler at the reported stack
Risks: Low
Signed-off-by: ShivaBhaskar_Challa@comcast.com

Change-Id: I9c5f9d840c5521eb50958bfb25395db8727d0751
(cherry picked from commit ed6a3154ab22666163c89de2d3344263c751cb7d)
Reason for change: Un register and Re register when
 we are creating the registration to avoid duplicate
 registration after Xconf set Re register and publish
 event update
Test Procedure: Check for the Error msg for duplicate
 registration, all the registered events should report
 as desgined
Risks: Low
Signed-off-by: <shivabhaskar_challa@Comcast.com>

Change-Id: Ic2a8005c711605810381af493152b16dad0ec1c2
Reason for change: When using parameters:
     {
         "type":"dataModel",
         "method":"subscribe",
         "reference" : "Device.WiFi.AccessPoint.1.X_RDK_deviceConnected",
         "use":"accumulate"
     }
in T2 profile, at connecting by WiFi to device AP, telemetry
module crash. Issue was in "strdup" funcion called with NULL
pointer as parameters.
Test Procedure: Prepare T2 profile with required parameter.
Connect by WiFi devices few times.
Expected result - Telemetry module still working.
Risks: Low
Signed-off-by: Marcin Tomczyk <marcin.tomczyk@cognizant.com>
Priority: P1
Change-Id: I2eb75eceaa7cfc1db4df2121bcd4c98c3a203259
(cherry picked from commit 8b857e3f9f36f8796e7a671a7bdfe0d929eb5672)
Reason for change:
replace dependency on glib-2 link list APIs with rdk_linkedlist.c
TinyRDK project has given birth to a remarkable creation  rdk_linkedlist.c.
Rather than relying on the glib-2 link list APIs we should use a
light weight version of rdk_linkedlist.c which takes lesser
memory than glib-2.
Test Procedure: Sanity.
Risks: None.
Singed-off-by: Andre McCurdy <armccurdy@gmail.com>
Priority: P1
Signed-off-by: Stephen Barrett <stephen_barrett@comcast.com>
Change-Id: I2602f87f12fbc989eb1725e61fd02f04c97caf6e
(cherry picked from commit 9ae88faf92530e68413a59e52f3e22cb6718a2eb)
…ng run time

Reason for change: Replaced pthread_join with pthread_detach to resolve the mmutex lock issues.
Test Procedure: Build and Verify whether the multiprofiles are working properly.
                when one profile reaches its activation timeout it should be removed properly from profilelist.
Risks: Medium
Priority: P1
Signed-off-by: pkathi542 <PriyaDharshini_Kathiravan@comcast.com>

Change-Id: I39045b599f4282ba8765f5c21f07410bd9a57926
(cherry picked from commit a7e5bd089ed7eb8f761b0958bebd0117d86e05c7)
Reason for change: To resolve the t2 crash from ProfileXConf_isNameEqual function
Test Procedure: Configure the multiprofiles in both cjson and msgpack format and check for crashes. Crashes shouldn't be observed.
Risks: Medium
Priority: P1
Signed-off-by: pkathi542 <PriyaDharshini_Kathiravan@comcast.com>

Change-Id: I211c7f735ae92702ce30506585b767e7ee3cbe74
(cherry picked from commit 8d9599282155edc2e5f2503db35db4a4f3a62c9d)
Reason for change: There seems to be a race condition
 between the threads when we are doing an uninit
 adding a varible to sync the threads
Test Procedure: Check the ticket for steps
Risks: Low
Signed-off-by:shivabhaskar_challa@comcast.com

Change-Id: Iee7e0bc3a26f0d22d0b8f7a3e7f4d706682b5616
(cherry picked from commit 4e1ff656f6493b445863eb41b3d5e0d99c95c7fc)
(cherry picked from commit b6746e6ebc794d57fd977124049be0b81201a9cb)
Reason for change: If scheduler is triggered no need
  to send interrupt
Test Procedure: Trigger interrupt just after multi profile
 Time out it should not go to hung state
Risks: Medium
Signed-off-by: ShivaBhaskar_Challa@comcast.com

Change-Id: I186caa867916fbe1676ede693d55d3aef04e9930
(cherry picked from commit 99f566d8c564e2787d67b5c335fc886583294f2c)
@shivabhaskar shivabhaskar requested a review from a team as a code owner February 11, 2025 19:09
@rdkcmf-jenkins
Copy link

Coverity Issue - Data race condition

Accessing "compTr181ParamMap" without holding lock "compParamMap". Elsewhere, "compTr181ParamMap" is written to with "compParamMap" held 3 out of 3 times.

Medium Impact, CWE-366
MISSING_LOCK

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at:
source/ccspinterface/rbusInterface.c:618

@rdkcmf-jenkins
Copy link

Coverity Issue - Data race condition

Accessing "compTr181ParamMap" without holding lock "compParamMap". Elsewhere, "compTr181ParamMap" is written to with "compParamMap" held 3 out of 3 times.

Medium Impact, CWE-366
MISSING_LOCK

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at:
source/ccspinterface/rbusInterface.c:1120

@@ -565,9 +564,11 @@
T2Info("Waiting for CollectAndReport to be complete : %s\n", singleProfile->name);
pthread_mutex_lock(&plMutex);
initialized=false;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverity Issue - Check of thread-shared field evades lock acquisition

Thread1 sets "initialized" to a new value. Now the two threads have an inconsistent view of "initialized" and updates to fields correlated with "initialized" may be lost.

High Impact, CWE-543
LOCK_EVASION

How to fix

Guard the modification of "initialized" and the read used to decide whether to modify "initialized" with the same set of locks.

if(count++ > 30){
break;
}
sleep(1);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverity Issue - Waiting while holding a lock

Call to "sleep" might sleep while holding lock "scMutex".

Medium Impact, CWE-667
SLEEP

@@ -1132,6 +1148,8 @@
T2Debug("Freeing compTr181ParamMap \n");
hash_map_destroy(compTr181ParamMap, freeComponentEventList);
compTr181ParamMap = NULL;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverity Issue - Check of thread-shared field evades lock acquisition

Thread1 sets "compTr181ParamMap" to a new value. Now the two threads have an inconsistent view of "compTr181ParamMap" and updates to fields of "compTr181ParamMap" or fields correlated with "compTr181ParamMap" may be lost.

High Impact, CWE-543
LOCK_EVASION

How to fix

Guard the modification of "compTr181ParamMap" and the read used to decide whether to modify "compTr181ParamMap" with the same set of locks.

strvalue[strlen(strvalue)-1] = '\0';
}
ret = report_or_cache_data(strvalue, marker);
if(strvalue != NULL){

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverity Issue - Dereference before null check

Null-checking "strvalue" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.

Medium Impact, CWE-476
REVERSE_INULL

if(mutex_return != 0){
T2Error("tProfile Mutex locking failed : %d \n",mutex_return);
if(mutex_return == EBUSY)
T2Error("tProfile Mutex is Busy, already the report generation might be in progress \n");
return T2ERROR_FAILURE;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverity Issue - Missing unlock

Returning without unlocking "scMutex".

Medium Impact, CWE-667
LOCK

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 3 committers have signed the CLA.

✅ mtomczykmobica
❌ mkamat
❌ PriyaKathiravan


mkamat seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants