SNTP broken in 3.5.2 with FreeRTOS+LwIP?

Tip / Sign in to post questions, reply, level up, and achieve exciting badges. Know more

cross mob
Anonymous
Not applicable

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!

0 Likes
1 Solution
AxLi_1746341
Level 7
Level 7
10 comments on KBA 5 comments on KBA First comment on KBA

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().

View solution in original post

0 Likes
9 Replies
Anonymous
Not applicable

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

0 Likes
Anonymous
Not applicable

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,

0 Likes
Anonymous
Not applicable

    /* 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;

    }

0 Likes

dpruessnermarkmendelsohn​ and axel.lin

Check your inbox for an invite from the community.

0 Likes

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.

0 Likes
AxLi_1746341
Level 7
Level 7
10 comments on KBA 5 comments on KBA First comment on KBA

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().

0 Likes
Anonymous
Not applicable

There is no socket created here

This fix is specific for the SNTP implementation

0 Likes

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.

0 Likes
Anonymous
Not applicable

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