Repository navigation
fix: add null pointer checks after malloc calls - #33
Merged
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds null checks and error handling after malloc calls in ipwatchd’s analyse and config paths to avoid dereferencing null pointers and improve behavior under low-memory conditions. Flow diagram for malloc error handling in ipwd_analyse conflic_mac allocationflowchart TD
Start["ipwd_analyse entry (conflict detection path)"] --> CheckConflict["Is IP conflict detected and rcv_smac present?"]
CheckConflict -->|No| End["Return or continue normal processing"]
CheckConflict -->|Yes| LogInfo["Log IP conflict info with ipwd_message IPWD_MSG_TYPE_INFO"]
LogInfo --> MallocConflicMac["Allocate check_context.conflic_mac with malloc"]
MallocConflicMac --> CheckConflicMacNull{Is check_context.conflic_mac NULL?}
CheckConflicMacNull -->|Yes| LogConflicMacError["Log error with ipwd_message IPWD_MSG_TYPE_ERROR (malloc failed for conflic_mac)"]
LogConflicMacError --> EarlyReturn["Return early from ipwd_analyse"]
CheckConflicMacNull -->|No| CopySmac["memcpy rcv_smac into check_context.conflic_mac"]
CopySmac --> ContinueProcessing["Continue normal packet analysis"]
EarlyReturn --> FunctionExit["ipwd_analyse exit"]
ContinueProcessing --> FunctionExit
Flow diagram for malloc error handling in ipwd_analyse newdevinfo allocationflowchart TD
Start["ipwd_analyse entry (device list processing path)"] --> ForEachDevice["Iterate over devices.dev array"]
ForEachDevice --> CheckExist["For current device, check exist flag"]
CheckExist -->|exist != 0| NextDevice["Skip allocation and check next device"]
CheckExist -->|exist == 0| MallocNewDevInfo["Allocate IPCONFLICT_DEV_INFO *newdevinfo with malloc"]
MallocNewDevInfo --> CheckNewDevInfoNull{Is newdevinfo NULL?}
CheckNewDevInfoNull -->|Yes| LogNewDevInfoError["Log error with ipwd_message IPWD_MSG_TYPE_ERROR (malloc failed for IPCONFLICT_DEV_INFO)"]
LogNewDevInfoError --> BreakLoop["Break out of device loop"]
CheckNewDevInfoNull -->|No| CopyIp["memcpy newdevinfo->ip from devices.dev[i].ip"]
CopyIp --> CopyMac["memcpy newdevinfo->mac from devices.dev[i].mac"]
CopyMac --> CopyRemoteMac["memcpy newdevinfo->remote_mac from rcv_smac"]
CopyRemoteMac --> LinkIntoList["Link newdevinfo into IP conflict info structure or list"]
LinkIntoList --> NextDevice
NextDevice -->|More devices| ForEachDevice
NextDevice -->|No more devices| FunctionExit["ipwd_analyse exit"]
BreakLoop --> FunctionExit
Flow diagram for malloc error handling in ipwd_read_config ipconflict_dev_info allocationflowchart TD
Start["ipwd_read_config entry"] --> InitDevices["Initialize devices.dev and set devices.devnum = 0"]
InitDevices --> MallocIpConflict["Allocate ipconflict_dev_info with malloc sizeof(IPCONFLICT_DEV_INFO)"]
MallocIpConflict --> CheckIpConflictNull{Is ipconflict_dev_info NULL?}
CheckIpConflictNull -->|Yes| LogIpConflictError["Log error with ipwd_message IPWD_MSG_TYPE_ERROR (malloc failed for ipconflict_dev_info)"]
LogIpConflictError --> ReturnError["Return IPWD_RV_ERROR from ipwd_read_config"]
CheckIpConflictNull -->|No| MemsetIp["memset ipconflict_dev_info->ip to 0"]
MemsetIp --> MemsetMac["memset ipconflict_dev_info->mac to 0"]
MemsetMac --> MemsetRemoteMac["memset ipconflict_dev_info->remote_mac to 0"]
MemsetRemoteMac --> ContinueConfig["Continue reading and parsing configuration file"]
ContinueConfig --> ReturnStatus["Return success or other status from ipwd_read_config"]
ReturnError --> FunctionExit["ipwd_read_config exit"]
ReturnStatus --> FunctionExit
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In ipwd_analyse, the early return after check_context.conflic_mac malloc failure may skip any cleanup or state-reset that this function is expected to perform for the current packet; consider routing failures through a single exit path that handles any required cleanup and keeps check_context/devices state consistent.
- The three malloc error paths use slightly different control-flow responses (return, break, return value) and hard-coded messages; consider standardizing the error-handling pattern and centralizing the error message format to make future changes and debugging easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In ipwd_analyse, the early return after check_context.conflic_mac malloc failure may skip any cleanup or state-reset that this function is expected to perform for the current packet; consider routing failures through a single exit path that handles any required cleanup and keeps check_context/devices state consistent.
- The three malloc error paths use slightly different control-flow responses (return, break, return value) and hard-coded messages; consider standardizing the error-handling pattern and centralizing the error message format to make future changes and debugging easier.
## Individual Comments
### Comment 1
<location> `src/plugin-sdbus/plugin-ipwatchd/analyse.c:261-265` </location>
<code_context>
if (exist == 0)
{
IPCONFLICT_DEV_INFO *newdevinfo = (IPCONFLICT_DEV_INFO *)malloc(sizeof(IPCONFLICT_DEV_INFO));
+ if (newdevinfo == NULL) {
+ ipwd_message (IPWD_MSG_TYPE_ERROR, "Unable to allocate memory for IPCONFLICT_DEV_INFO - malloc failed");
+ break;
+ }
memcpy (newdevinfo->ip, devices.dev[i].ip, IPWD_MAX_DEVICE_ADDRESS_LEN);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Breaking out of the loop on allocation failure may silently skip processing remaining devices.
When `newdevinfo == NULL`, you log an error and `break`, which prevents processing any remaining devices in this call while still returning as if processing largely succeeded. If an allocation failure should be fatal, consider returning an error or otherwise aborting the whole operation; if not, use `continue` so other devices are still checked for conflicts.
```suggestion
IPCONFLICT_DEV_INFO *newdevinfo = (IPCONFLICT_DEV_INFO *)malloc(sizeof(IPCONFLICT_DEV_INFO));
if (newdevinfo == NULL) {
ipwd_message (IPWD_MSG_TYPE_ERROR, "Unable to allocate memory for IPCONFLICT_DEV_INFO - malloc failed");
return;
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Added null pointer checks after malloc calls in three locations to prevent potential segmentation faults and improve error handling. When malloc fails to allocate memory, the code now logs an appropriate error message and returns early instead of proceeding with null pointers. 1. Added check for check_context.conflic_mac allocation in ipwd_analyse function 2. Added check for newdevinfo allocation in ipwd_analyse function 3. Added check for ipconflict_dev_info allocation in ipwd_read_config function Influence: 1. Test memory allocation failure scenarios to verify error messages are logged correctly 2. Verify that null pointer checks prevent crashes when malloc fails 3. Test normal operation to ensure memory allocation still works correctly 4. Check error handling behavior when system is under memory pressure fix: 为malloc调用添加空指针检查 在三个位置为malloc调用添加了空指针检查，以防止潜在的段错误并改进错误处 理。当malloc分配内存失败时，代码现在会记录适当的错误消息并提前返回，而不 是继续使用空指针。 1. 在ipwd_analyse函数中添加了对check_context.conflic_mac分配的检查 2. 在ipwd_analyse函数中添加了对newdevinfo分配的检查 3. 在ipwd_read_config函数中添加了对ipconflict_dev_info分配的检查 Influence: 1. 测试内存分配失败场景，验证错误消息是否正确记录 2. 验证当malloc失败时空指针检查是否能防止崩溃 3. 测试正常操作以确保内存分配仍然正常工作 4. 检查系统内存压力下的错误处理行为
deepin pr auto review我来对这段代码进行审查和分析：
建议：
建议：
// 定义统一的内存分配错误处理函数
static void* safe_malloc(size_t size, const char* error_msg) {
void* ptr = malloc(size);
if (ptr == NULL) {
ipwd_message(IPWD_MSG_TYPE_ERROR, "%s - malloc failed", error_msg);
return NULL;
}
return ptr;
}
// 使用示例
check_context.conflic_mac = safe_malloc(IPWD_MAX_DEVICE_ADDRESS_LEN,
"Unable to allocate memory for conflic_mac");
if (check_context.conflic_mac == NULL) {
return;
}
// 或者使用calloc
check_context.conflic_mac = calloc(1, IPWD_MAX_DEVICE_ADDRESS_LEN);
if (check_context.conflic_mac == NULL) {
ipwd_message(IPWD_MSG_TYPE_ERROR,
"Unable to allocate memory for conflic_mac - calloc failed");
return;
}
总的来说，这次改进主要提升了代码的安全性，通过添加内存分配失败的检查和处理，使程序更加健壮。建议继续完善错误处理机制，并考虑代码的复用性。 |
yixinshark
approved these changes
Nov 25, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mhduiy, yixinshark The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added null pointer checks after malloc calls in three locations to prevent potential segmentation faults and improve error handling. When malloc fails to allocate memory, the code now logs an appropriate error message and returns early instead of proceeding with null pointers.
Influence:
fix: 为malloc调用添加空指针检查
在三个位置为malloc调用添加了空指针检查，以防止潜在的段错误并改进错误处
理。当malloc分配内存失败时，代码现在会记录适当的错误消息并提前返回，而不
是继续使用空指针。
Influence:
Summary by Sourcery
Add defensive handling for memory allocation failures in the IP watch daemon to avoid crashes and improve error reporting.
Bug Fixes: