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

Wrongly identified mbedTLS memory configuration (IDFGH-10399) #57

Closed
HamzaHajeir opened this issue Jun 12, 2023 · 1 comment
Closed

Comments

@HamzaHajeir
Copy link

Hi there,
I've had an issue that forced me to debug deeply last couple of days.

I'm using ALTCP_TLS_MBEDTLS, have failed to get a pcb out of calling altcp_tls_new(), that results in mbedtls fails to allocate memory even if very large memory was available, printing to the log:

IDF/components/mbedtls/mbedtls/library/ssl_tls.c:3857: alloc(16717 bytes) failed
mbedtls_ssl_setup failed

It starts by the user call [altcp_tls_create_config_server](https://github.com/lwip-tcpip/lwip/blob/e29870c15e8bf28eac9c811dd236c474f3f2008f/src/apps/altcp_tls/altcp_tls_mbedtls.c#LL863C33-L863C33), which creates tls config first, which in turn initializes memory by altcp_mbedtls_mem_init, which overrides mbedtls calloc/free.

Which its implementation compares against LWIP defined macro (MEM_SIZE), which is defaulted to 1600,

Overriding it is protected by the preprocessor ALTCP_MBEDTLS_PLATFORM_ALLOC but it holds a wrong dealing with mbedtls configuration.

MbedTLS has three states of memory configuration:

  • normal calloc()/free(), which can be easily overriden when MBEDTLS_PLATFORM_MEMORY is defined.
    /* After defining MBEDTLS_PLATFORM_MEMORY it's possible to override mbedtls's calloc/free by calling mbedtls_platform_set_calloc_free().
  • Standard calloc/free, which goes ESP's defaulted heap_caps_calloc()/heap_caps_free() if a custom macro isn't defined CONFIG_MBEDTLS_CUSTOM_MEM_ALLOC (header) (source)
  • User-defined macros MBEDTLS_PLATFORM_{CALLOC,FREE}_MACRO.

So what's proposed is to add a check to the preprocessor to become:

#if defined(MBEDTLS_PLATFORM_MEMORY) && \
     !defined(MBEDTLS_PLATFORM_FREE_MACRO) && \
    !defined(MBEDTLS_PLATFORM_STD_CALLOC)

Keep in mind that mbedTLS checks for misconfiguration (missing the correlated free/calloc or mixing std-defined with platform-defined):
https://github.com/espressif/mbedtls/blob/15b55d406db3918bac88aaf5ef2c6e036d1e0f0e/include/mbedtls/check_config.h#L470-L496

@github-actions github-actions bot changed the title Wrongly identified mbedTLS memory configuration Wrongly identified mbedTLS memory configuration (IDFGH-10399) Jun 12, 2023
@david-cermak
Copy link
Collaborator

I agree that the condition on enabling ALTCP_MBEDTLS_PLATFORM_ALLOC is probably wrong, but since you've already reported the problem upstream in https://savannah.nongnu.org/patch/index.php?10368 and we won't support altcp in IDF in the near future, I think we can close this issue.

We could possibly cherry-pick 601e1bb to Espressif stable branches, but that's another issue and going to happen soon, once we switch to the latest lwip release.

There're several ways to work around this issue, you already mentioned redefining MEM_SIZE. Alternatively you can add your implementation to altcp_tls_mbedtls_mem.h and remove altcp_tls_mbedtls_mem.c from your sources.

Please reopen if needed.

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