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

socket: tcp application crashes when there are no more net buffers in case of reception #33516

Closed
psanyi opened this issue Mar 19, 2021 · 8 comments
Labels
bug The issue is a bug, or the PR is fixing a bug

Comments

@psanyi
Copy link

psanyi commented Mar 19, 2021

Tcp application crashes when receiving a huge packet and there are no more net buffers.

I reproduced this error on a Nucleo-144 (STM32F429) development board.

Steps to reproduce the behavior:
Prepare the board with a net app (e.g. civetweb)

  1. cd zephyr
  2. west build samples/net/civetweb/http_server -b nucleo_f429zi
  3. west flash

Testing procedure:

  1. Create a huge file with random content:

dd if=/dev/random of=huge_file count=10

  1. Send the file using netcat

nc 172.16.100.3 8080 < huge_file

Expected behavior
The stack should wait until the buffer frees or drop the packages to
prevent an overflow

Logs and console output
image

Environment:

  • OS: Ubuntu Linux
  • Toolchain Zephyr SDK,
  • Commit 856ff7d
@psanyi psanyi added the bug The issue is a bug, or the PR is fixing a bug label Mar 19, 2021
@gudnimg
Copy link
Contributor

gudnimg commented Mar 19, 2021

I ran west build samples/net/civetweb/http_server -b nucleo_f429zi to check what version of civetweb zephyr downloads. On my end I got v1.11.0 but there is a newer version v1.13.0 available. v1.11 to v1.13 includes a lot of changes: https:/civetweb/civetweb/blob/master/RELEASE_NOTES.md

I checked this viewing line 44 in modules/lib/civetweb/CMakeLists.txt and printing out the version number confirms Zephyr is using 1.11.0

I would try to manually update to 1.13.0 to see if there's a change in your issue. I'm not sure how to change this in Zephyr itself since this Cmake file is only generated when I build.

This is how the Cmake code looks like on my end for reference:

set(CIVETWEB_VERSION "1.11.0" CACHE STRING "The version of the civetweb library")
string(REGEX MATCH "([0-9]+)\\.([0-9]+)\\.([0-9]+)" CIVETWEB_VERSION_MATCH "${CIVETWEB_VERSION}")
if ("${CIVETWEB_VERSION_MATCH}" STREQUAL "")
  message(FATAL_ERROR "Must specify a semantic version: major.minor.patch")
endif()
set(CIVETWEB_VERSION_MAJOR "${CMAKE_MATCH_1}")
set(CIVETWEB_VERSION_MINOR "${CMAKE_MATCH_2}")
set(CIVETWEB_VERSION_PATCH "${CMAKE_MATCH_3}")
message(STATUS "Found Civetweb version v${CMAKE_MATCH_1}.${CMAKE_MATCH_2}.${CMAKE_MATCH_3}") # <-- I added this line

@psanyi
Copy link
Author

psanyi commented Mar 19, 2021

@GunZi200 thank you for the quick replay. I tested also the modbus-tcp server with the same procedure and it works without any issue. It seems that the civetwebserver has some flaws.

Cmake file at my side

set(CIVETWEB_VERSION "1.11.0" CACHE STRING "The version of the civetweb library")
string(REGEX MATCH "([0-9]+)\\.([0-9]+)\\.([0-9]+)" CIVETWEB_VERSION_MATCH "${CIVETWEB_VERSION}")
if ("${CIVETWEB_VERSION_MATCH}" STREQUAL "")
  message(FATAL_ERROR "Must specify a semantic version: major.minor.patch")
endif()
set(CIVETWEB_VERSION_MAJOR "${CMAKE_MATCH_1}")
set(CIVETWEB_VERSION_MINOR "${CMAKE_MATCH_2}")
set(CIVETWEB_VERSION_PATCH `"${CMAKE_MATCH_3}")`

@psanyi
Copy link
Author

psanyi commented Mar 19, 2021

I have updated civetweb from https:/civetweb/civetweb
commit b7b3d2cdd94052204b98b2c6260207f97db29acc

The problem still exists
image

Cmake file

set(CIVETWEB_VERSION "1.14.0" CACHE STRING "The version of the civetweb library")
string(REGEX MATCH "([0-9]+)\\.([0-9]+)\\.([0-9]+)" CIVETWEB_VERSION_MATCH "${CIVETWEB_VERSION}")
if ("${CIVETWEB_VERSION_MATCH}" STREQUAL "")
  message(FATAL_ERROR "Must specify a semantic version: major.minor.patch")
endif()
set(CIVETWEB_VERSION_MAJOR "${CMAKE_MATCH_1}")
set(CIVETWEB_VERSION_MINOR "${CMAKE_MATCH_2}")
set(CIVETWEB_VERSION_PATCH "${CMAKE_MATCH_3}")

@gudnimg
Copy link
Contributor

gudnimg commented Mar 20, 2021

Ok, I created a separate issue to update the version. #33520

Your issue is something else. Are you sure this is not a bug on civetweb’s end? Where in the code does the buffer overflow take place? I don’t have the board to test myself.

Hopefully someone more familiar with the code can help you.

@psanyi
Copy link
Author

psanyi commented Mar 20, 2021

Ok. I did a small research on this issue.

As it seems, civetweb causing the overflow and is not related to socket api. I will close this issue.
For security reasons (requirements), I have to drop this package and search for an alternative.
Here is the stack trace.

#0  0x61000000 in ?? ()
#1  0x0800789c in k_queue_get (timeout=..., queue=<optimized out>)
    at zephyr/include/generated/syscalls/kernel.h:538
#2  tcp_in (conn=<unavailable>, pkt=<optimized out>)
    at /home/sanyi/zephyr/zephyr/subsys/net/ip/tcp2.c:1955
#3  0x080063c2 in pkt_alloc_buffer (timeout=..., size=74, pool=0x20004bf4 <tx_bufs>)
    at /home/sanyi/zephyr/zephyr/subsys/net/ip/net_pkt.c:867
#4  net_pkt_alloc_buffer (pkt=pkt@entry=0x2001914c <_k_mem_slab_buf_tx_pkts+836>, size=size@entry=26, 
    proto=proto@entry=IPPROTO_TCP, timeout=...) at /home/sanyi/zephyr/zephyr/subsys/net/ip/net_pkt.c:1159
#5  0x08010762 in pkt_alloc_with_buffer (slab=slab@entry=0x20004ae8 <tx_pkts>, 
    iface=0x20004c1c <__net_if_dts_ord_33_0>, size=size@entry=26, family=<optimized out>, 
    proto=IPPROTO_TCP, timeout=timeout@entry=...)
    at /home/sanyi/zephyr/zephyr/subsys/net/ip/net_pkt.c:1411
#6  0x08006462 in net_pkt_alloc_with_buffer (iface=<optimized out>, size=size@entry=26, 
    family=<optimized out>, proto=<optimized out>, timeout=...)
    at /home/sanyi/zephyr/zephyr/subsys/net/ip/net_pkt.c:1442
#7  0x0800fffe in context_alloc_pkt (timeout=..., len=26, context=0x200087cc <contexts+124>)
    at /home/sanyi/zephyr/zephyr/include/net/net_context.h:594
#8  context_sendto (context=context@entry=0x200087cc <contexts+124>, 
    buf=buf@entry=0x2000aea8 <work_q_stack+456>, len=len@entry=26, 
    dst_addr=dst_addr@entry=0x200087f0 <contexts+160>, addrlen=<optimized out>, cb=cb@entry=0x0, 
    user_data=user_data@entry=0x0, sendto=sendto@entry=false, timeout=...)
    at /home/sanyi/zephyr/zephyr/subsys/net/ip/net_context.c:1624
--Type <RET> for more, q to quit, c to continue without paging--
#9  0x080102cc in net_context_send (context=context@entry=0x200087cc <contexts+124>, 
    buf=buf@entry=0x2000aea8 <work_q_stack+456>, len=len@entry=26, cb=cb@entry=0x0, timeout=..., user_data=0x0)
    at /home/sanyi/zephyr/zephyr/subsys/net/ip/net_context.c:1805
#10 0x08002814 in zsock_sendto_ctx (ctx=0x200087cc <contexts+124>, buf=0x2000aea8 <work_q_stack+456>, len=26, 
    flags=<optimized out>, dest_addr=0x0, addrlen=0)
    at /home/sanyi/zephyr/zephyr/subsys/net/lib/sockets/sockets.c:614
#11 0x0800e49c in z_impl_zsock_sendto (sock=sock@entry=4, buf=0x2000aea8 <work_q_stack+456>, 
    buf@entry=0x800e49d <z_impl_zsock_sendto+56>, len=len@entry=26, flags=flags@entry=0, 
    dest_addr=dest_addr@entry=0x0, addrlen=addrlen@entry=0)
    at /home/sanyi/zephyr/zephyr/subsys/net/lib/sockets/sockets.c:657
#12 0x0800d508 in zsock_sendto (dest_addr=0x0, addrlen=0, flags=flags@entry=0, len=len@entry=26, 
    buf=buf@entry=0x800e49d <z_impl_zsock_sendto+56>, sock=sock@entry=4)
    at zephyr/include/generated/syscalls/socket.h:150
#13 zsock_send (flags=flags@entry=0, len=len@entry=26, buf=buf@entry=0x800e49d <z_impl_zsock_sendto+56>, 
    sock=sock@entry=4) at /home/sanyi/zephyr/zephyr/include/net/socket.h:345
#14 send (sock=sock@entry=4, buf=buf@entry=0x2000aea8 <work_q_stack+456>, len=len@entry=26, flags=flags@entry=0)
    at ../src/libc_extensions.c:189
#15 0x080011f0 in push_inner (fp=0x0, timeout=30, len=26, 
    buf=0x2000aea8 <work_q_stack+456> "HTTP/1.0 400 Bad Request\r\n", ssl=<optimized out>, sock=<optimized out>, 
    ctx=<optimized out>) at /home/sanyi/zephyr/modules/lib/civetweb/src/civetweb.c:6011
#16 push_all (ctx=0x20000068 <z_malloc_heap_mem+72>, sock=4, ssl=0x0, 
    buf=buf@entry=0x2000aea8 <work_q_stack+456> "HTTP/1.0 400 Bad Request\r\n", len=<optimized out>, 
    len@entry=26, fp=0x0) at /home/sanyi/zephyr/modules/lib/civetweb/src/civetweb.c:6113
#17 0x08012164 in mg_write (conn=conn@entry=0x20000680 <z_malloc_heap_mem+1632>, 
    buf=0x2000aea8 <work_q_stack+456>, len=len@entry=26)
    at /home/sanyi/zephyr/modules/lib/civetweb/src/civetweb.c:6713
#18 0x080121a0 in mg_vprintf (conn=conn@entry=0x20000680 <z_malloc_heap_mem+1632>, 
    fmt=fmt@entry=0x80160f1 "1.0", ap=..., ap@entry=...)
    at /home/sanyi/zephyr/modules/lib/civetweb/src/civetweb.c:6878
#19 0x080121c8 in mg_printf (conn=conn@entry=0x20000680 <z_malloc_heap_mem+1632>, 
    fmt=0x80160fb "HTTP/%s %i %s\r\n") at /home/sanyi/zephyr/modules/lib/civetweb/src/civetweb.c:6895
#20 0x0800981e in send_http1_response_status_line (conn=0x20000680 <z_malloc_heap_mem+1632>)
--Type <RET> for more, q to quit, c to continue without paging--
   /modules/lib/civetweb/src/response.inl:58
#21 mg_response_header_send (conn=0x20000680 <z_malloc_heap_mem+1632>)
    at /home/sanyi/zephyr/modules/lib/civetweb/src/response.inl:293
#22 0x0800995c in mg_send_http_error_impl (conn=conn@entry=0x20000680 <z_malloc_heap_mem+1632>, status=400, 
    fmt=fmt@entry=0x20000680 <z_malloc_heap_mem+1632> "\001", args=..., args@entry=...)
    at /home/sanyi/zephyr/modules/lib/civetweb/src/civetweb.c:4496
#23 0x080121e2 in mg_send_http_error (conn=conn@entry=0x20000680 <z_malloc_heap_mem+1632>, status=<optimized out>, 
    fmt=0x8015a56 "%s") at /home/sanyi/zephyr/modules/lib/civetweb/src/civetweb.c:4521
#24 0x08009ffc in process_new_connection (conn=0x20000680 <z_malloc_heap_mem+1632>)
    at /home/sanyi/zephyr/modules/lib/civetweb/src/civetweb.c:18359
#25 worker_thread_run (conn=0x20000680 <z_malloc_heap_mem+1632>, conn@entry=<error reading variable: value has been optimized out>)
    at /home/sanyi/zephyr/modules/lib/civetweb/src/civetweb.c:18850
#26 worker_thread (thread_func_param=0x20000680 <z_malloc_heap_mem+1632>, 
    thread_func_param@entry=<error reading variable: value has been optimized out>)
    at /home/sanyi/zephyr/modules/lib/civetweb/src/civetweb.c:18911
#27 0x0800efc6 in zephyr_thread_wrapper (arg1=<optimized out>, arg2=<optimized out>, arg3=<optimized out>)
    at /home/sanyi/zephyr/zephyr/lib/posix/pthread.c:119
#28 0x0800d5e0 in z_thread_entry (entry=0x800efc3 <zephyr_thread_wrapper>, p1=<optimized out>, p2=<optimized out>, 
    p3=<optimized out>) at /home/sanyi/zephyr/zephyr/lib/os/thread_entry.c:29
#29 0x00375282 in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

@gudnimg
Copy link
Contributor

gudnimg commented Mar 20, 2021

@psanyi I've been looking into Zephyr's version of Civetweb more... I discovered that Zephyr uses this repo: https:/zephyrproject-rtos/civetweb

I looked into the history and am wondering if this commit (which is not used by Zephyrs revision) may help you?:

zephyrproject-rtos/civetweb@3565451 specifically see this issue which looks very similar to yours: #27020 it was never resolved completely.

I know Zephyr does not use this commit because this is what the compiled size looks like for your board:

Memory region         Used Size  Region Size  %age Used
           FLASH:      121408 B         2 MB      5.79%
             CCM:          0 GB        64 KB      0.00%
            SRAM:       95199 B       192 KB     48.42%
        IDT_LIST:          0 GB         2 KB      0.00%

After zephyrproject-rtos/civetweb@3565451:

Memory region         Used Size  Region Size  %age Used
           FLASH:      123392 B         2 MB      5.88%
             CCM:          0 GB        64 KB      0.00%
            SRAM:      119775 B       192 KB     60.92%
        IDT_LIST:          0 GB         2 KB      0.00%

But then again you tested with v1.14 and this did not fix the issue for you.

Maybe some of these sizes in the sample's prj.conf are too small?

CONFIG_MINIMAL_LIBC_MALLOC_ARENA_SIZE=16384
CONFIG_NET_TX_STACK_SIZE=2048
CONFIG_NET_RX_STACK_SIZE=2048
CONFIG_ISR_STACK_SIZE=2048
CONFIG_MAIN_STACK_SIZE=2048
CONFIG_IDLE_STACK_SIZE=1024

@bel2125
Copy link

bel2125 commented Mar 20, 2021

Regarding stack sizes (this issue and the corresponding issue on CivetWeb civetweb/civetweb#981).

Please either use a stack size of at least 16 kB, or shrink other buffers as well: MG_BUF_LEN, the max_request_size default setting. And check what features you want to activate (Lua, CGI, WebSocket, SSL), since deactivating unused features will also reduce the footprint. Documentation:
https:/civetweb/civetweb/blob/master/docs/Embedding.md#stack-sizes
https:/civetweb/civetweb/blob/master/docs/Building.md#setting-build-options
civetweb/civetweb#837

Important: Reducing the stack size below 16 kB does not work without reducing MG_BUF_LEN.
A stack with 8 kB will probably work with MG_BUF_LEN and max_request_size of 4 kB, if the web application does not use excessively long cookies or URLs. This requires some testing and fine tuning.

@psanyi
Copy link
Author

psanyi commented Mar 20, 2021

Here is my complete project configuration.

#kernel
CONFIG_TICKLESS_KERNEL=n
#CONFIG_TICKLESS_IDLE=n
CONFIG_EXTRA_EXCEPTION_INFO=y
CONFIG_DEBUG_COREDUMP=y
#CONFIG_DEBUG_COREDUMP_MEMORY_DUMP_MIN=y

CONFIG_LOG=y
CONFIG_LOG_MODE_MINIMAL=y

CONFIG_NET_LOG=y
CONFIG_MODBUS_LOG_LEVEL_DBG=y

CONFIG_CIVETWEB=y
CONFIG_JSON_LIBRARY=y
CONFIG_NET_TCP_ISN_RFC6528=n

# pthreads
CONFIG_POSIX_API=y
CONFIG_PTHREAD_IPC=y
CONFIG_POSIX_MQUEUE=y

# libc
CONFIG_MINIMAL_LIBC_MALLOC_ARENA_SIZE=16384

# gpio
CONFIG_GPIO=y

# Networking config
CONFIG_NETWORKING=y
CONFIG_NET_IPV4=y
CONFIG_NET_IPV6=n
CONFIG_NET_TCP=y
CONFIG_NET_SOCKETS=y

# CONFIG_NET_SOCKETS_POSIX_NAMES=y
CONFIG_NET_TX_STACK_SIZE=2048
CONFIG_NET_RX_STACK_SIZE=2048
CONFIG_ISR_STACK_SIZE=2048
CONFIG_MAIN_STACK_SIZE=2048
CONFIG_IDLE_STACK_SIZE=1024

# Network address config
CONFIG_NET_CONFIG_SETTINGS=y
CONFIG_NET_CONFIG_NEED_IPV4=y
CONFIG_NET_CONFIG_MY_IPV4_GW="172.16.100.1"
CONFIG_NET_CONFIG_MY_IPV4_ADDR="172.16.100.3"
CONFIG_NET_CONFIG_MY_IPV4_NETMASK="255.255.0.0"
CONFIG_NET_CONFIG_PEER_IPV4_ADDR="172.16.100.1"

CONFIG_MODBUS=y
CONFIG_MODBUS_ROLE_SERVER=y
CONFIG_MODBUS_RAW_ADU=y
CONFIG_MODBUS_NUMOF_RAW_ADU=1

Footprint

[1/1] Linking C executable zephyr/zephyr.elf
Memory region         Used Size  Region Size  %age Used
           FLASH:      115064 B         2 MB      5.49%
             CCM:          0 GB        64 KB      0.00%
            SRAM:      116544 B       192 KB     59.28%
        IDT_LIST:          0 GB         2 KB      0.00%

Civet server config options

#define MAX_REQUEST_SIZE_BYTES			1024

	static const char * const options[] = {
		"listening_ports",
		STRINGIFY(HTTP_PORT),
		"num_threads",
		"1",
		"max_request_size",
		STRINGIFY(MAX_REQUEST_SIZE_BYTES),
		NULL
	};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

No branches or pull requests

3 participants