-
Notifications
You must be signed in to change notification settings - Fork 52
RDKEMW-12282: Fix Coverity identified issues - dobby #408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request addresses Coverity static analysis issues identified in the Dobby container runtime. The changes primarily focus on fixing concurrency issues, resource management problems, TOCTOU (time-of-check-time-of-use) vulnerabilities, buffer overflows, and other code quality issues.
Changes:
- Fixed concurrency and thread safety issues by adding proper mutex locking, using atomic variables where appropriate, and ensuring locks are released before callbacks
- Addressed resource leaks and proper initialization of member variables across multiple plugins
- Replaced TOCTOU-prone patterns (access/stat followed by open/opendir) with direct operations
- Fixed buffer overflow vulnerabilities by using strncpy instead of strcpy and adding bounds checking
- Added exception handling in destructors and main functions to prevent unexpected terminations
- Fixed iterator invalidation issues and improved error handling throughout the codebase
Reviewed changes
Copilot reviewed 50 out of 51 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/source/DobbyUtils.cpp | Added mutex locking for thread-safe access to metadata maps in const methods |
| utils/include/DobbyUtils.h | Made metadata mutex mutable to allow locking in const methods |
| settings/source/Settings.cpp | Fixed resource leak by calling wordfree and corrected indentation |
| rdkPlugins/Storage/source/RefCountFile.cpp | Added overflow checks for reference counter and improved error handling |
| rdkPlugins/Storage/source/LoopMountDetails.cpp | Prevented double close by resetting fd after close |
| rdkPlugins/Storage/source/DynamicMountDetails.cpp | Fixed file creation logic and improved error handling |
| rdkPlugins/OOMCrash/source/OOMCrashPlugin.cpp | Improved file operation error handling and fixed indentation |
| rdkPlugins/Networking/source/NetworkingPlugin.cpp | Initialized mPluginData pointer to nullptr |
| rdkPlugins/Networking/source/NetworkSetup.cpp | Fixed iterator usage to avoid dangling iterator issues |
| rdkPlugins/Networking/source/IPAllocator.cpp | Replaced stat with opendir to fix TOCTOU vulnerability |
| rdkPlugins/Minidump/source/AnonymousFile.cpp | Fixed file size check and corrected indentation |
| rdkPlugins/Logging/source/FileSink.cpp | Initialized mFileSizeLimit member variable |
| rdkPlugins/IONMemory/source/IonMemoryPlugin.cpp | Initialized mPluginData pointer to nullptr |
| rdkPlugins/HttpProxy/source/HttpProxyPlugin.cpp | Initialized mPluginData pointer to nullptr |
| rdkPlugins/AppServices/source/AppServicesRdkPlugin.cpp | Initialized mPluginConfig pointer to nullptr |
| plugins/OpenCDM/source/OpenCDMPlugin.cpp | Replaced access() with direct operations and improved error handling |
| plugins/MulticastSockets/source/MulticastSocketsPlugin.cpp | Added socket validation, fixed format specifier, and used move semantics |
| plugins/EthanLog/source/EthanLogLoop.cpp | Added lock guard for thread-safe access to mClients |
| plugins/EthanLog/source/EthanLogClient.cpp | Added bounds checking to prevent buffer overflow |
| plugins/EthanLog/client/cat/ethanlog-cat.cpp | Fixed buffer overflow check and changed mBufferOffset type to size_t |
| plugins/Common/source/ServiceMonitor.cpp | Improved lock management for state change notifications |
| pluginLauncher/tool/source/Main.cpp | Added missing break statement in switch case |
| pluginLauncher/lib/source/DobbyRdkPluginUtils.cpp | Initialized exitStatus member variable |
| pluginLauncher/lib/source/DobbyRdkPluginManager.cpp | Added scandir error handling and removed incorrect close call |
| pluginLauncher/lib/include/DobbyRdkPluginUtils.h | Added lock guard for thread-safe access to annotations |
| ipcUtils/source/DobbyIpcBus.cpp | Fixed lock guard scoping |
| daemon/process/source/Main.cpp | Added exception handling in main and missing break statement |
| daemon/lib/source/include/DobbyWorkQueue.h | Changed counters to atomic types |
| daemon/lib/source/DobbyWorkQueue.cpp | Added missing lock guard |
| daemon/lib/source/DobbyStats.cpp | Fixed format specifier for long long type |
| daemon/lib/source/DobbyManager.cpp | Fixed iterator invalidation, lambda captures, and added exception annotations |
| daemon/lib/source/DobbyLogger.cpp | Added exception handling in destructor and replaced strcpy with strncpy |
| daemon/lib/source/DobbyLogRelay.cpp | Initialized member variables and replaced strcpy with strncpy |
| daemon/lib/source/DobbyContainer.cpp | Initialized mRestartCount member variable |
| daemon/lib/source/Dobby.cpp | Improved error handling for reply sends and removed unreachable code |
| client/tool/source/Main.cpp | Fixed TOCTOU vulnerability, added missing break, and lock guards |
| client/lib/source/DobbyProxy.cpp | Fixed lock guard scoping |
| bundle/tool/source/Main.cpp | Added exception handling in main and missing break statement |
| bundle/lib/source/DobbyTemplate.cpp | Fixed potential use-after-unlock by copying pointer before unlock |
| bundle/lib/source/DobbySpecConfig.cpp | Initialized variables and added lock guard |
| bundle/lib/source/DobbyRootfs.cpp | Replaced access with open to fix TOCTOU vulnerability |
| bundle/lib/source/DobbyConfig.cpp | Added lock guard and fixed indentation |
| bundle/lib/source/DobbyBundleConfig.cpp | Added lock guards for const methods and fixed indentation |
| bundle/lib/include/DobbyBundleConfig.h | Fixed indentation |
| AppInfrastructure/ReadLine/source/ReadLine.cpp | Fixed escape sequence in format string |
| AppInfrastructure/Public/Common/Notifier.h | Removed extra unlock and added Coverity annotation |
| AppInfrastructure/IpcService/source/sdbus/SDBusIpcService.cpp | Fixed integer overflow in timeout calculations |
| AppInfrastructure/Common/source/Timer.cpp | Added exception handling in destructor |
| AppInfrastructure/Common/source/ThreadedDispatcher.cpp | Improved lock management and fixed indentation |
| AppInfrastructure/Common/include/IDGenerator.h | Replaced rand() with random_device for better randomness |
| AppInfrastructure/Common/include/ConditionVariable.h | Removed unreachable return statement |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else if (errno == EEXIST) | ||
| { | ||
| AI_LOG_INFO("File '%s' already present, skipping creation", filePath.c_str()); | ||
| } |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function returns false when the file already exists (EEXIST), but this should be considered a success case since the goal is to ensure the file exists. The function should return true after logging the info message on line 278 to match the function's purpose of "write file if not exists".
bundle/tool/source/Main.cpp
Outdated
| // And we're done | ||
| AICommon::termLogging(); | ||
| return EXIT_SUCCESS; | ||
| } catch (const std::exception& e) { |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The closing brace on line 285 is misplaced - it should close the try block, not be on the same line as the return statement. The catch block will not properly catch exceptions from lines 232-284 with this brace placement. The closing brace should be on line 284 (after "return EXIT_SUCCESS;"), and line 285 should just have the catch statement.
| } catch (const std::exception& e) { | |
| } | |
| catch (const std::exception& e) { |
Description
RDKEMW-12282: Fix Coverity identified issues - dobby
If there is a corresponding JIRA ticket, please ensure it is in the title of the PR.
Test Procedure
How to test this PR (if applicable)
Type of Change
Requires Bitbake Recipe changes?
meta-rdk/recipes-containers/dobby/dobby.inc) must be modified to support the changes in this PR (beyond updatingSRC_REV)