<div data-ntes="ntes_mail_body_root" style="line-height:1.7;color:#000000;font-size:14px;font-family:Arial"><p style="margin:0;"></p><p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">I'm renmingshuai's successor, a beginner with dnsmasq, and this e-mail is follow-up to</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt"><a href="https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2024q3/017664.html">https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2024q3/017664.html</a></p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">Question 1: Why does the dhcp_reply function add the stack variable netid to daemon->dhcp_conf->netid->list, which is a global variable?</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">dhcp_reply</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">if (config)</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">{</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">struct dhcp_netid_list *list;</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt"> </p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">for (list = config->netid; list; list = list->next)</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">{</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">list->list->next = netid;</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">netid = list->list;</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">}</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">}</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">}</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">When we find config, 'list->list->next = netid;' add the stack variable netid to the end of the linked list daemon->dhcp_conf->netid->list. According to my understanding, the code is intended to add the first node of config->netid->list to the head of netid. The purpose of 'list->list->next = netid;' is to cache netid so that after 'netid = list->list;' the new netid->next is the original netid. Do I understand that right?</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">If I understand this correctly, the issue is that we add the stack variable (for example, netid = &iface_id;, the &iface_id is a stack address) to daemon->dhcp_conf->netid->list, and when dnsmasq receives the SIGHUP signal, 'clear_dynamic_conf' will free the memory of daemon->dhcp_conf->netid. In this case, daemon->dhcp_conf->netid in stack space is freed as a pointer. If the pointer is not an address returned by malloc, a bad-free error may occur, resulting in a segmentation fault.</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">The coredump is as follows:</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">[New LWP 2925451]</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">[Thread debugging using libthread_db enabled]</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">Using host libthread_db library "/usr/lib64/libthread_db.so.1".</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">Core was generated by `dnsmasq --no-hosts --no-resolv --pid-file=/var/lib/neutron/dhcp/d7435322-bf42-4'.</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">Program terminated with signal SIGSEGV, Segmentation fault.</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">#0 0x0000151e5168b0be in free () from /usr/lib64/libc.so.6</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">(gdb) bt</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">#0 0x0000151e5168b0be in free () from /usr/lib64/libc.so.6</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">#1 0x00005651025a08f0 in dhcp_netid_free (nid=0xc08b255501ae7f00) at option.c:1022</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">#2 dhcp_netid_list_free (netid=0x0) at option.c:1053</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">#3 dhcp_config_free (config=0x565104124bc0) at option.c:1071</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">#4 0x00005651025aa011 in clear_dynamic_conf () at option.c:5204</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">#5 reread_dhcp () at option.c:5245</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">#6 0x00005651025b2c53 in clear_cache_and_reload (now=1722067132) at dnsmasq.c:1699</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">#7 0x00005651025960ac in async_event (now=1722067132, pipe=15) at dnsmasq.c:1449</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">#8 main (argc=<optimized out>, argv=<optimized out>) at dnsmasq.c:1192</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">(gdb)</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt"> </p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">Question 2: Why does the dhcp_reply function use two netid linked lists: netid and tagif_netid? Can't we just use one?</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">Through code review, my understanding is as follows: netid stores various tags. tagif_netid matches the content of the --tag-if option with the tag saved in netid. If the matching is successful, the tag in 'set:' is added and returned to tagif_netid. But why divide it into two linked lists? Can't I directly return it to netid?</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">The reason I'm asking this question is, I want to put all the similar</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">list->list->next = netid;</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">netid = list->list;</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">change to</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">netid = dhcp_netid_create(list->list->net, netid);</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">The purpose of this is to avoid adding the stack variable netid to any global variable. Then call </p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">dhcp_netid_free(netid);</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">before the function return to free all the memory that has been malloced.(I plan to change return to goto to avoid manually freeing memory at each return)</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">If both the netid and tagif_netid exist, the free logic is complex.</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">If question one is indeed a problem, is the above modification reasonable? Is there a better modification?</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt"> </p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">Question 3: Are there any common test cases for dnsmasq?</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt">I found this test case on github, but it covered so little that I wanted to find a comprehensive use case to ensure that my changes were correct and did not introduce problems.</p> <p style="margin:0in;font-family:"Microsoft YaHei";font-size:11.0pt"><a href="https://github.com/InfrastructureServices/dnsmasq-tests">https://github.com/InfrastructureServices/dnsmasq-tests</a></p><br></div><div><br></div><div><br></div><br><div><br></div><div><br></div><div><br></div><div><br></div><div id="imail_signature" class="ntes-signature"></div>