Repository navigation
Memory leak in 'msgparse.c' in 'parse_edns_options_from_query(...)' #1324
Description
Dynamic memory allocated in 'msgparse.c' by calling 'edns_opt_list_append(...)' at lines: 989, 1032, 1094, 1130, 1142; and lost at the end of the scope.
In 'edns_opt_list_append(...)' dynamic memory is allocated for new element 'struct edns_option* opt' by calling 'regional_alloc(...)' and is not freed.
Activity
wenxuan70 commentedon Aug 30, 2025
In fact, the outer
parse_edns_from_query_pktusesworker->scratchpadas region. When it fails, all memory blocks will be released.unbound/daemon/worker.c
Line 1672 in 44da5ee
- Fix for #1324: Fix to free edns options scratch in ratelimit case.
wcawijngaards commentedon Sep 1, 2025
It looks like both posts are right. The memory from edns_opt_list_append is allocated in the region. That region is freed with the region later, in the daemon/worker.c file. After searching through the code to make sure, I saw a case in the worker.c file where the region with the edns opt list was not freed correctly.
The commit fixes that the region, where the edns opt list is allocated, is freed in a case with ratelimit. That would stop memory from leaking into the region, it is freed every time.
I am not sure if I caught the issue that is being reported, though, since the allocation in a region is freed later. Is there something more that needs to be fixed, for the memory leak? If the issue is resolved, that is fine as well. It is nice that a bug was fixed already :-) .
TimofeyGritsun commentedon Sep 2, 2025
Thanks!
There is one more thing:
In "configparser.y" at lines 1170-1173 after checking for NULL pointer 'cfg_parser->cfg->http_endpoint' is dereferenced.
It may lead to UB, since 'yyeror("out of memory") does not stop the execution, to my knowledge.
wcawijngaards commentedon Sep 2, 2025
That is fixed in the commit, so it does not dereference the null pointer. The code is improved for the error case, where the error does not stop the execution there. Thank you for reporting that issue!
Merge remote-tracking branch 'nlnet/master'