SNTP broken in 3.5.2 with FreeRTOS+LwIP?

[Anonymous]

Can someone please confirm that 3.5.2 has an error in the SNTP code in sntp_get_time(...) (libraries/protocols/SNTP/sntp.c:113)?  Here, sntp_get_time(...) calls wiced_packet_create_udp( &socket, ...) , but socket is uninitialized (just random stack data).  The LwIP code (WICED/network/LwIP/WICED/tcpip.c:664) checks the socket->dtls_context against NULL and dereferences socket->dtls_context (see tcpip.c:669), causing a hard fault.

I saw a similar (but more subtle) error in libraries/protocols/HTTP/http.c:60 in wiced_http_get(...).  socket is initialized by wiced_create_socket(...), but wiced_create_socket(...) (WICED/network/LwIP/WICED/tcpip.c:254) does not zero out the structure memory.  In fact, it leaves socket->tls_context as a pointer to whatever random data the stack had previously.  So the wiced_tcp_connect(&socket, ...) call (http.c:64) fails at when the socket->tls_context is accessed through wiced_tcp_start_tls(socket) call inside wiced_tcp_connect(...) (tcpip.c:1034).

As a user of the API, I expect wiced_create_socket(...) to fully initialize the socket structure to default (ie, non-TLS) operation.  Only wiced_enable_tls(&socket) should modify the socket->tls_context variable and change it to a non-NULL value.-- right?  I found I can fix the wiced_http_get(...) function and pull it into my application by adding a memset(&socket, 0, sizeof(socket)) call before wiced_create_socket(...).

Most of my ported code works in spite of these issues.  But I'm guessing that is because in my application, most connections come from a global .bss memory space and are initialized to zero by libc.

I can fix these problems in the SDK, but I'm concerned about making too many modifications to code outside of my /apps/..., as the code will get clobbered by changes in future SDK versions.

Comments/confirmation of these errors would be appreciated.  Or (if unconfirmed), just tell me how to get SNTP working reliably.  Thanks!

[Anonymous]

Yes. This is correct. SNTP is broken there and this has been fixed in the next rev of the SDK to be released - 3.6.2

[Anonymous]

Thanks for the quick response.  I can bring it internal to my application and fix for now.  When do you expect 3.6.2 will be released?  Is 3.5.2 the latest to be working with?

Best regards,

[Anonymous]

    /* Create the query packet */
    memset( &socket, 0, sizeof(wiced_udp_socket_t));
    result = wiced_packet_create_udp( &socket, sizeof(ntp_packet_t), &packet, (uint8_t**) &data, &available_data_length );
    if ( ( result != WICED_SUCCESS ) || ( available_data_length < sizeof(ntp_packet_t) ) )
    {
        return WICED_ERROR;
    }

[MichaelF_56]

dpruessner​ markmendelsohn​ and axel.lin​

Check your inbox for an invite from the community.

[AxLi_1746341]

nsankar wrote:

Yes. This is correct. SNTP is broken there and this has been fixed in the next rev of the SDK to be released - 3.6.2

Hi nsankar

As this bug causes a hard fault.

Is it possible to post a patch to the forum ASAP since you already have a fix.

So users don't have to wail until next release?

Thanks.

[AxLi_1746341 · Accepted solution]

dpruessner wrote:

I saw a similar (but more subtle) error in libraries/protocols/HTTP/http.c:60 in wiced_http_get(...).  socket is initialized by wiced_create_socket(...), but wiced_create_socket(...) (WICED/network/LwIP/WICED/tcpip.c:254) does not zero out the structure memory.  In fact, it leaves socket->tls_context as a pointer to whatever random data the stack had previously.  So the wiced_tcp_connect(&socket, ...) call (http.c:64) fails at when the socket->tls_context is accessed through wiced_tcp_start_tls(socket) call inside wiced_tcp_connect(...) (tcpip.c:1034).

As a user of the API, I expect wiced_create_socket(...) to fully initialize the socket structure to default (ie, non-TLS) operation.  Only wiced_enable_tls(&socket) should modify the socket->tls_context variable and change it to a non-NULL value.-- right?  I found I can fix the wiced_http_get(...) function and pull it into my application by adding a memset(&socket, 0, sizeof(socket)) call before wiced_create_socket(...).

I think had better to add

memset(socket, 0, sizeof(*socket));

in the begin of wiced_tcp_create_socket() implementation.

So you don't need to add memset to all users of wiced_tcp_create_socket().

[Anonymous]

There is no socket created here

This fix is specific for the SNTP implementation

[AxLi_1746341]

Your fix is for SNTP implementation.

But Do you see dpruessner's original post:

I saw a similar (but more subtle) error in libraries/protocols/HTTP/http.c:60 in wiced_http_get(...).  socket is initialized by wiced_create_socket(...), but wiced_create_socket(...) (WICED/network/LwIP/WICED/tcpip.c:254) does not zero out the structure memory.  In fact, it leaves socket->tls_context as a pointer to whatever random data the stack had previously.  So the wiced_tcp_connect(&socket, ...) call (http.c:64) fails at when the socket->tls_context is accessed through wiced_tcp_start_tls(socket) call inside wiced_tcp_connect(...) (tcpip.c:1034).

PS. My previous post includes the related quote.

[Anonymous]

Sorry for some reason it did not show up in my browser

Yes that will be fixed as well

That fix is already in there for NetX and NetX_Duo but was missed for LWIP