TCXB8-3898 Facing kernel panic reboot due to System OOM #8
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request addresses kernel panic/OOM issues by cleaning up test configuration files in /tmp/cci_res/ after test completion. The cleanup is triggered using shell commands at two key points in the test lifecycle: when results are uploaded successfully or when tests are aborted.
Changes:
- Added cleanup calls to remove all files in
/tmp/cci_res/directory after test completion - Cleanup occurs in both the success path (after uploading results) and failure path (after aborting tests)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| m_state = wlan_emu_tests_state_cmd_wait; | ||
| dml_state = wlan_emu_dml_tests_state_complete_failure; | ||
| } | ||
| system("rm -rf /tmp/cci_res/*"); |
There was a problem hiding this comment.
The return value of the system() call is not being checked. This means failures in cleaning up the directory (e.g., permissions issues, disk errors, or if /tmp/cci_res/ doesn't exist) will go unnoticed. Consider checking the return value and logging any failures to aid in debugging.
| m_state = wlan_emu_tests_state_cmd_wait; | ||
| dml_state = wlan_emu_dml_tests_state_complete_failure; | ||
| m_ui_mgr.update_log_file_offset(); | ||
| system("rm -rf /tmp/cci_res/*"); |
There was a problem hiding this comment.
The return value of the system() call is not being checked. This means failures in cleaning up the directory (e.g., permissions issues, disk errors, or if /tmp/cci_res/ doesn't exist) will go unnoticed. Consider checking the return value and logging any failures to aid in debugging.
| m_state = wlan_emu_tests_state_cmd_wait; | ||
| dml_state = wlan_emu_dml_tests_state_complete_failure; | ||
| } | ||
| system("rm -rf /tmp/cci_res/*"); |
There was a problem hiding this comment.
Using system("rm -rf /tmp/cci_res/*") introduces a security risk as the command is executed through the shell. This is vulnerable to command injection if the path could be influenced by external inputs. Additionally, using shell commands for file operations is less portable and harder to handle errors for. Consider using C++ filesystem operations (std::filesystem::remove_all) or POSIX functions (like rmdir, unlink in a loop) for safer and more controlled directory cleanup. Several files in the codebase use unlink() for file removal (e.g., src/external_agent_cci/wlan_ext_emu.cpp:985, src/ui/wlan_emu_ui_mgr.cpp:3918), which demonstrates a safer pattern already in use.
| m_state = wlan_emu_tests_state_cmd_wait; | ||
| dml_state = wlan_emu_dml_tests_state_complete_failure; | ||
| m_ui_mgr.update_log_file_offset(); | ||
| system("rm -rf /tmp/cci_res/*"); |
There was a problem hiding this comment.
Using system("rm -rf /tmp/cci_res/*") introduces a security risk as the command is executed through the shell. This is vulnerable to command injection if the path could be influenced by external inputs. Additionally, using shell commands for file operations is less portable and harder to handle errors for. Consider using C++ filesystem operations (std::filesystem::remove_all) or POSIX functions (like rmdir, unlink in a loop) for safer and more controlled directory cleanup. Several files in the codebase use unlink() for file removal (e.g., src/external_agent_cci/wlan_ext_emu.cpp:985, src/ui/wlan_emu_ui_mgr.cpp:3918), which demonstrates a safer pattern already in use.
2 participants
Impacted Platforms:
All RDKB Platforms
Reason for change: Removing the test configs once test completed
Test Procedure: Run number of tests and check if /tmp/cci_res/ folder is cleared post completion
Risks: None
Priority: P0