Skip to content

memory leak in mbedtls using net_app DTLS client #8631

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

Closed
mazhenke opened this issue Jun 29, 2018 · 6 comments
Closed

memory leak in mbedtls using net_app DTLS client #8631

mazhenke opened this issue Jun 29, 2018 · 6 comments
Assignees
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@mazhenke
Copy link

I am using the DTLS client in zephyr, and I found there is memory leak when run the following flow mutli times:

  1. net_app_init_udp_client
  2. net_app_set_cb
  3. net_app_client_tls
  4. net_app_connect
  5. net_app_close
  6. net_app_release
  7. go to step1 and repeat

Seems the mebedtls_ssl_xxx_init functions in _net_app_tls_init function doesn't release the heap when calling net_app_release

@mateusz-
Copy link
Contributor

I think I am also running into this. If I call mqtt_connect a couple of times I end up getting an error in my setup cert callback. Specifically, mbedtls_x509_crt_parse_der fails with MBEDTLS_ERR_X509_ALLOC_FAILED.

My callback is straight out of the mqtt example file.

@nashif nashif added the bug The issue is a bug, or the PR is fixing a bug label Jul 3, 2018
@mike-scott
Copy link
Contributor

mike-scott commented Jul 4, 2018 via email

@nashif nashif added the priority: medium Medium impact/importance bug label Jul 13, 2018
@pfalcon
Copy link
Collaborator

pfalcon commented Aug 21, 2018

Similar comment to what was posted to #8746 :

Recently, DTLS socket support was merged: #9338 , and that's going to be the new official API for (D)TLS usage. You are thus recommended to switch to it whenever possible. You may find that older net_app DTLS client is in "maintenance mode" (that actually depends on developers' time availability, and AFAIK, they're fully occupied). I thus lower priority of this ticket.

@pfalcon
Copy link
Collaborator

pfalcon commented Aug 21, 2018

@mike-scott :

I have a WIP patchset which includes this hack to fix the leak you're describing. I need some time to submit an actual fix: 82c5d82#diff-88ec0070db0157cb49aeb6609e210f1aR3883

Definitely please submit a fix if you have that investigated. Thanks.

@pfalcon pfalcon changed the title memory leak in mbedtls using DTLS client memory leak in mbedtls using net_app DTLS client Aug 21, 2018
@pfalcon pfalcon added priority: low Low impact/importance bug and removed priority: medium Medium impact/importance bug labels Aug 21, 2018
@pfalcon pfalcon assigned mike-scott and unassigned pfalcon Aug 21, 2018
@tautologyclub
Copy link
Contributor

I'm guessing all net_app-dtls issues will be more or less left to hang henceforth?

That's understandable I guess, but I'd make sure to mark it as __deprecated if so.

We discovered a really weird bug today where net_app registered a rx callback which was called by one of the handshake functions, but the rx callback itself indirectly called the handshake function itself - thus recursing itself (silently, for some reason) into oblivion.

Our current intuition says that we should probably forget about net_app and migrate to sockets because it seems like net_app is a lot more __deprecated than the documentation suggests. True?

@jukkar
Copy link
Member

jukkar commented Feb 25, 2019

net-app is gone, so closing this issue

@jukkar jukkar closed this as completed Feb 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

No branches or pull requests

7 participants