RDKB-65948: [8.6p1s2]Observed CcspLMLite Crash due to Presence Detection with Fingerprint 44028269 - #83
RDKB-65948: [8.6p1s2]Observed CcspLMLite Crash due to Presence Detection with Fingerprint 44028269#83im1308 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent a CcspLMLite crash caused by stale (use-after-free) host instance handles being cached by the CCSP framework and later dereferenced in Host_GetParamStringValue().
Changes:
- Adds
IsValidHostHandle()to validatehInsContext(asPLmObjectHost) againstlmHosts.hostArray[]underLmHostObjectMutex. - Adds a guard in
Host_GetParamStringValue()to bail out early when a stale host handle is detected. - Updates
GetParamStringValue_common()to usestrnlen()(but this currently breaks required-size reporting semantics).
Comments suppressed due to low confidence (1)
source/lm/cosa_hosts_dml.c:156
strnlen(value, *pUlSize)breaks the TR-181/CCSP GetParamStringValue contract for required-size reporting: when the string is longer than the buffer,strnlenreturns*pUlSize, so the code only bumps*pUlSizeby 1 instead of returning the full required length. Callers that reallocate and retry once will still fail for long strings.
len = strnlen(value, *pUlSize);
if (len >= *pUlSize)
{
*pUlSize = len + 1;
rc = 1;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| return FALSE; | ||
| } |
There was a problem hiding this comment.
I feel it is not required because for each get call, it will iterate over all the Hosts.
NULL check and mutex lock is sufficient.
There was a problem hiding this comment.
My understanding is that the mutex protects concurrent access to lmHosts, but it doesn't validate the lifetime of the hInsContext received from the framework. Since Host_GetEntry() returns the host pointer after releasing the mutex, my concern is that the handle may become stale if the host is removed before a later Host_GetParamStringValue() callback. IsValidHostHandle() is intended to verify that the received handle still belongs to the current hostArray before dereferencing it. If there is an existing guarantee that host objects are never freed (or that hInsContext is always refreshed or reference-counted), then I'm happy to remove the check.
| CcspTraceDebug(("%s:%d, unlocked LmHostObjectMutex\n",__FUNCTION__,__LINE__)); | ||
| return (ULONG) -1; | ||
| } | ||
|
|
There was a problem hiding this comment.
In this file, Lot of other places also needs to be handled apart from this function.
There was a problem hiding this comment.
This has been handled. Please review the changes
|
📋 PR Format Reminder
Expected: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
source/lm/cosa_hosts_dml.c:156
GetParamStringValue_common()now has a stray block afterstrnlen(...), which results in anelsewithout a matchingif(compile error) and also breaks the buffer-size logic. If the intent is to preserve the original "return 1 and set *pUlSize to required size" behavior, the size check needs to be restored (and usingstrlen()here is appropriate once the handle is validated elsewhere).
len = strnlen(value, *pUlSize);
{
*pUlSize = len + 1;
rc = 1;
}
RDKB-65948: [8.6p1s2]Observed CcspLMLite Crash due to Presence Detection with Fingerprint 44028269
RootCause and Fix:
The crash at len = strlen(value) is a stale-handle use-after-free. Host_GetEntry returns the row handle after releasing LmHostObjectMutex; the CCSP framework caches it and later calls Host_GetParamStringValue. If the LM background thread deletes/reallocates that host in between, pHost->pStringParaValue[i] is a non-NULL garbage pointer, so strlen faults. The value == NULL check can't catch it.
Adding a new helper IsValidHostHandle() to verify that the PLmObjectHost received through hInsContext still exists in lmHosts.hostArray[].
The validation is performed while holding LmHostObjectMutex, before dereferencing the host object.
If the handle is no longer valid, the function logs a warning, unlocks the mutex, and returns an error instead of dereferencing a potentially stale pointer.
Purpose: Prevent access to a stale PLmObjectHost in case the host entry was removed or replaced after Host_GetEntry() returned the handle.