Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DHCP re-init leak #489

Open
ThomasNauwelaerts opened this issue Sep 20, 2018 · 5 comments
Open

DHCP re-init leak #489

ThomasNauwelaerts opened this issue Sep 20, 2018 · 5 comments

Comments

@ThomasNauwelaerts
Copy link

When the network connection is lost, the device must re-init the DHCP and request an ip to the DHCP server. When removing our ethernet cable from the device, than plugging it back in, we do 'pico_dhcp_initiate_negotiation' again. At that point +-400 bytes are leaking.

@ThomasNauwelaerts
Copy link
Author

@jelledevleeschouwer ?

@jelledevleeschouwer
Copy link
Contributor

@ThomasNauwelaerts, this should be investigated. Could you reproduce this issue on a host (i.e. linux) environment?

@ThomasNauwelaerts
Copy link
Author

@jelledevleeschouwer , I'm not able to use a host environment.
Problem is, I'm not sure how to re-init DHCP, for example when the device is changed to another network, or the network changes. Now it just did the 'pico_dhcp_initiate_negotiation' (after 'pico_dhcp_client_abort'), but that's leaking memory.
It also seems to be impossible to detect if the current IP address isn't valid anymore?

@szemzoa
Copy link

szemzoa commented Mar 26, 2020

The leak is coming from the function pico_dhcp_server_add_negotiation().
When the client renew the lease, the new request comes with a different xid.
The pico_dhcp_server_find_negotiation() will fail with the new xid as a key, and new struct pico_dhcp_server_negotiation will be inserted to the tree every time.

I don't know if this is totally ok or not, but looking up the entries with the hwaddr and deleting the old entry solves the memleak, something like this:

static void pico_dhcp_server_free_negotiation_hwaddr(struct pico_stack *S, struct pico_dhcp_hdr *hdr)
{
struct pico_dhcp_server_negotiation *dhcpn = NULL;
struct pico_tree_node *idx;

pico_tree_foreach(idx, &S->DHCPNegotiations) {
    dhcpn = idx->keyValue;
    if (memcmp(hdr->hwaddr, dhcpn->hwaddr.addr, PICO_SIZE_ETH) == 0) {
        pico_tree_delete(&S->DHCPNegotiations, dhcpn);
        PICO_FREE(dhcpn);
        break;
    }
}

}

@szemzoa
Copy link

szemzoa commented Mar 26, 2020

oops, this one was for the DHCP server part, which is leaks too, I will try to find the client leak too...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants