[Dnsmasq-discuss] netid list issue

[Simon Kelley]

On 20/03/13 10:42, Alexander Nickulin wrote:
> Hi, Simon and all!
>
> I have some issues concerning to dhcp_reply at rfc2131.c
>
> struct dhcp_netid *netid points to local (stack) variables
> known_id, iface_id, cpewan_id that will be lost after subj
> function returns.
>
> So i have after
>    if (config)
>      {
>        struct dhcp_netid_list *list;
>
>        for (list = config->netid; list; list = list->next)
>          {
>            list->list->next = netid;
>            netid = list->list;
>          }
>      }
>
> and before dhcp match loop:
>
> config: 0x649400    <- malloc'd
>    netid: 0x6494c0   <- malloc'd
>      list: 0x6494a0  <- malloc'd
>        net: 0x6494e0 "red"  <- tag supplied by me using dhcp-conf opt, malloc'd
>        next: 0x7fffffffe220 <- stack known_id variable! (x86_64)
>          net: 0x4416ea "known" <- .rodata
>          next: 0x7fffffffe210  <- stack iface_id
>            net: 0x7fffffffe650 "eth1" <- stack iface_name variable
>            next: 0x0
>      next: 0x0
>
> May be rfc3315.c (DHCPv6) has the same issue. I dont use it.
>
> This is so obvious that i'm not sure. Can i understand anything wrong?
> But of course SIGSEGV appears on dhcp_config->netid->list iterating
> outside of subj function:
> for (id = netid->list; id; id = id->next)
>
> I guess this is not ok.


Please can you post to the list, or send directly to me, the 
configuration which  causes a crash?

Of course a SEGV is not OK, but I can't see a problem with the existing 
code. The lifetime of the linked-list you show above is the  lifetime of 
the dhcp_reply() function, so the fact that some of the storage 
evaporates after that doesn't matter. The pointer to the head of the 
list has gone too, and the ->next fields in the long-lived netid 
stuctures will be overwritten before they are next used.

Note that

for ( list= netid->list; list; list = list->next)

is _not_ iterating over the same linked-list. It's going down a linked 
list if dhcp_netid_list containing all the tags set in the dhcp-host 
configuration and finding the dhcp_netid's pointed to there, These are 
to the linked-list of current tags.

list->list->next is not the same as list->next

This is ludicrously complicated linked-list bashing, I realise. Sorry, 
it's just how my brain works.


Cheers,

Simon.

> Thanks!
>
> _______________________________________________
> Dnsmasq-discuss mailing list
> Dnsmasq-discuss at lists.thekelleys.org.uk
> http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
>