uIP: bus-fault arising from use of TCP in an echo-client; reason: uip_periodic() called with a NULL buf


Rohit Grover
 

Hello Community,

This follows from an earlier post on users@: https://lists.zephyrproject.org/archives/list/users(a)lists.zephyrproject.org/thread/PAL5NBO4UF6XEH3PN7YCW5YQVC2XCR5X/

I see a bus-fault arising from use of TCP over uIP. It seems that a periodic invocation of uip_periodic() from net_timer_fiber [to advance uIP's state machine] is being passed a NULL pointer. uip_periodic() has been defined in uip.h to index into a field of the buf, causing a bus-fault:

<code>
/**
* Periodic processing for a connection identified by its number.
*
* This function does the necessary periodic processing (timers,
* polling) for a uIP TCP connection, and should be called when the
* periodic uIP timer goes off.
*
* The usual way of calling the function is through a for() loop like
* this:
\code
for(i = 0; i < UIP_CONNS; ++i) {
uip_periodic(i);
if(uip_len > 0) {
devicedriver_send();
}
}
\endcode
*
* \param conn The number of the connection which is to be periodically polled.
*
* \hideinitializer
*/
#if UIP_TCP
#define uip_periodic(buf, conn) do { uip_set_conn(buf) = &uip_conns[conn]; \
uip_process(&buf, UIP_TIMER); } while (0)

</code>

uip_periodic() is getting called with a NULL 'buf' from eventhandler() [in net/ip/contiki/ip/tcpip.c] in the case of a PROCESS_EVENT_TIMER.

Here's the call stack leading to eventhandler():
net_timer_fiber() --> etimer_request_poll () --> process_post_sync(&etimer_process, PROCESS_EVENT_POLL, NULL, NULL) --> ... --> tcpip_process() --> eventhandler()

It can be seen from the above call-chain that process_post_sync() always calls tcpip_process() with a NULL buf. This is incoherent with the definition of uip_periodic(buff, conn), which indexes into the buf.

Please note that according to the comment header above uip_periodic(), it is meant to receive only a connection-id, but this is inconsistent with its implementation. The implementation was last changed by SHA d0ba82a5 by Jukka Rissanen on 07/07/2015 13:45 to add the additional buf pointer. That commit says:

Initial TCP support

Initial import for some new TCP related files from Contiki.
Fixed the compilation of imported TCP code but actual TCP
support comes in following commits.

It seems that there is an inconsistency somewhere.
I'm able to reproduce this using a simple zephyr-based echo-client which attempts to connect to an external echo-server. The bus fault happens during the first connection-establishment exchanges. I believe this should show up for any use of TCP over uIP.

regards,
Rohit.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Jukka Rissanen
 

Hi Rohit,

there is a fix for this at
https://gerrit.zephyrproject.org/r/#/c/4050/

Please test if possible.

Cheers,
Jukka

On Fri, 2016-08-12 at 09:49 +0000, Rohit Grover wrote:
Hello Community,

This follows from an earlier post on users@: https://lists.zephyrproj
ect.org/archives/list/users(a)lists.zephyrproject.org/thread/PAL5NBO4UF
6XEH3PN7YCW5YQVC2XCR5X/

I see a bus-fault arising from use of TCP over uIP. It seems that a
periodic invocation of uip_periodic() from net_timer_fiber [to
advance uIP's state machine] is being passed a NULL pointer.
uip_periodic() has been defined in uip.h to index into a field of the
buf, causing a bus-fault:

<code>
/**
 * Periodic processing for a connection identified by its number.
 *
 * This function does the necessary periodic processing (timers,
 * polling) for a uIP TCP connection, and should be called when the
 * periodic uIP timer goes off.
 *
 * The usual way of calling the function is through a for() loop like
 * this:
 \code
 for(i = 0; i < UIP_CONNS; ++i) {
 uip_periodic(i);
 if(uip_len > 0) {
 devicedriver_send();
 }
 }
 \endcode
 *
 * \param conn The number of the connection which is to be
periodically polled.
 *
 * \hideinitializer
 */
#if UIP_TCP
#define uip_periodic(buf, conn) do { uip_set_conn(buf) =
&uip_conns[conn]; \
    uip_process(&buf, UIP_TIMER); } while (0)

</code>

uip_periodic() is getting called with a NULL 'buf' from
eventhandler() [in net/ip/contiki/ip/tcpip.c] in the case of a
PROCESS_EVENT_TIMER.

Here's the call stack leading to eventhandler():
net_timer_fiber() -->  etimer_request_poll () -->
process_post_sync(&etimer_process, PROCESS_EVENT_POLL, NULL, NULL) --
... --> tcpip_process() --> eventhandler()
It can be seen from the above call-chain that process_post_sync()
always calls tcpip_process() with a NULL buf. This is incoherent with
the definition of uip_periodic(buff, conn), which indexes into the
buf.

Please note that according to the comment header above
uip_periodic(), it is meant to receive only a connection-id, but this
is inconsistent with its implementation. The implementation was last
changed by SHA d0ba82a5 by Jukka Rissanen on 07/07/2015 13:45 to add
the additional buf pointer. That commit says:

    Initial TCP support

    Initial import for some new TCP related files from Contiki.
    Fixed the compilation of imported TCP code but actual TCP
    support comes in following commits.

It seems that there is an inconsistency somewhere.
I'm able to reproduce this using a simple zephyr-based echo-client
which attempts to connect to an external echo-server. The bus fault
happens during the first connection-establishment exchanges. I
believe this should show up for any use of TCP over uIP.

regards,
Rohit.
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended
recipient, please notify the sender immediately and do not disclose
the contents to any other person, use it for any purpose, or store or
copy the information in any medium. Thank you.


Rohit Grover
 

Thanks for the fix. I no longer get a bus fault. I still don’t see client's data being received at the echo-server, but that needs to be looked at with fresh eyes.

Please share if you have a working demo which uses TCP; it would be useful addition to samples/net.

Regards,
Rohit.

-----Original Message-----
>From: Jukka Rissanen [mailto:jukka.rissanen(a)linux.intel.com]
>Sent: 12 August 2016 14:52
>To: Rohit Grover; devel(a)lists.zephyrproject.org
>Subject: [devel] Re: uIP: bus-fault arising from use of TCP in an echo-client;
>reason: uip_periodic() called with a NULL buf
>
>Hi Rohit,
>
>there is a fix for this at
>https://gerrit.zephyrproject.org/r/#/c/4050/
>
>Please test if possible.
>
>Cheers,
>Jukka
>
>
>On Fri, 2016-08-12 at 09:49 +0000, Rohit Grover wrote:
>> Hello Community,
>>
>> This follows from an earlier post on users@: https://lists.zephyrproj
>> ect.org/archives/list/users(a)lists.zephyrproject.org/thread/PAL5NBO4UF
>> 6XEH3PN7YCW5YQVC2XCR5X/
>>
>> I see a bus-fault arising from use of TCP over uIP. It seems that a
>> periodic invocation of uip_periodic() from net_timer_fiber [to advance
>> uIP's state machine] is being passed a NULL pointer.
>> uip_periodic() has been defined in uip.h to index into a field of the
>> buf, causing a bus-fault:
>>
>> <code>
>> /**
>> * Periodic processing for a connection identified by its number.
>> *
>> * This function does the necessary periodic processing (timers,
>> * polling) for a uIP TCP connection, and should be called when the
>> * periodic uIP timer goes off.
>> *
>> * The usual way of calling the function is through a for() loop like
>> * this:
>> \code
>> for(i = 0; i < UIP_CONNS; ++i) {
>> uip_periodic(i);
>> if(uip_len > 0) {
>> devicedriver_send();
>> }
>> }
>> \endcode
>> *
>> * \param conn The number of the connection which is to be
>> periodically polled.
>> *
>> * \hideinitializer
>> */
>> #if UIP_TCP
>> #define uip_periodic(buf, conn) do { uip_set_conn(buf) =
>> &uip_conns[conn]; \
>> uip_process(&buf, UIP_TIMER); } while (0)
>>
>> </code>
>>
>> uip_periodic() is getting called with a NULL 'buf' from
>> eventhandler() [in net/ip/contiki/ip/tcpip.c] in the case of a
>> PROCESS_EVENT_TIMER.
>>
>> Here's the call stack leading to eventhandler():
>> net_timer_fiber() --> etimer_request_poll () -->
>> process_post_sync(&etimer_process, PROCESS_EVENT_POLL, NULL, NULL) --
>> > ... --> tcpip_process() --> eventhandler()
>>
>> It can be seen from the above call-chain that process_post_sync()
>> always calls tcpip_process() with a NULL buf. This is incoherent with
>> the definition of uip_periodic(buff, conn), which indexes into the
>> buf.
>>
>> Please note that according to the comment header above uip_periodic(),
>> it is meant to receive only a connection-id, but this is inconsistent
>> with its implementation. The implementation was last changed by SHA
>> d0ba82a5 by Jukka Rissanen on 07/07/2015 13:45 to add the additional
>> buf pointer. That commit says:
>>
>> Initial TCP support
>>
>> Initial import for some new TCP related files from Contiki.
>> Fixed the compilation of imported TCP code but actual TCP
>> support comes in following commits.
>>
>> It seems that there is an inconsistency somewhere.
>> I'm able to reproduce this using a simple zephyr-based echo-client
>> which attempts to connect to an external echo-server. The bus fault
>> happens during the first connection-establishment exchanges. I believe
>> this should show up for any use of TCP over uIP.
>>
>> regards,
>> Rohit.
>> IMPORTANT NOTICE: The contents of this email and any attachments are
>> confidential and may also be privileged. If you are not the intended
>> recipient, please notify the sender immediately and do not disclose
>> the contents to any other person, use it for any purpose, or store or
>> copy the information in any medium. Thank you.
>>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Paul Sokolovsky
 

Hello Rohit,

On Fri, 12 Aug 2016 15:09:28 +0000
Rohit Grover <Rohit.Grover(a)arm.com> wrote:

Thanks for the fix. I no longer get a bus fault. I still don’t see
client's data being received at the echo-server, but that needs to be
looked at with fresh eyes.

Please share if you have a working demo which uses TCP; it would be
useful addition to samples/net.
There's already TCP support in samples/net/echo_server , you just need
to build it with CONFIG_NETWORKING_WITH_TCP=y . While waiting for
FRDM-K64F Ethernet driver progress, I decided to try QEMU virtual
networking. After some fiddling (which I'm going to describe in a
following mail), and defining the setting above, both UDP and TCP for
echo_server work for me:

UDP:
$ echo foobar | nc -6 -u 2001:db8::2 4242

raboof

TCP:

$ echo foobar | nc -6 -q3 2001:db8::2 4242
foobar

(unlike UDP server, TCP one doesn't reverse data order).

Pings also work:

$ ping6 2001:db8::2
PING 2001:db8::2(2001:db8::2) 56 data bytes
64 bytes from 2001:db8::2: icmp_seq=1 ttl=64 time=4.41 ms
64 bytes from 2001:db8::2: icmp_seq=2 ttl=64 time=2.50 ms
64 bytes from 2001:db8::2: icmp_seq=3 ttl=64 time=2.72 ms


Regards,
Rohit.

--
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog