NULL pointer access in zsock_getsockname_ctx with TCP2

[armandciejak]

Describe the bug
When testing civetweb sample application with TCP2 I noticed a NULL pointer access in memcpy when called in zsock_getsockname_ctx. I added asserts into memcpy because I already had similar problems and it helped catching the faulty code. Without the assert the error get unnoticed.

Note: The problem is not present in TCP1.

To Reproduce
Steps to reproduce the behavior:

1. Add asserts to memcpy: __ASSERT(d != NULL, "d is NULL"); and __ASSERT(s != NULL, "s is NULL");
2. west build --board <YOUR BOARD> zephyr/samples/net/sockets/civetweb -p
3. west flash
4. curl http://192.168.1.241:8080
5. See the assert on the console

The error is systematic. I tried it on both mimxrt1050_evk and nucleo_f767zi and I got the same results.

Expected behavior
No NULL pointer access.

Impact
This is a show stopper for my project since we use civetweb and are migrating to Zephyr v2.4 because we need all the improvements of TCP2.

Logs and console output

ASSERTION FAIL [s != ((void *)0)] @ WEST_TOPDIR/zephyr/lib/libc/minimal/source/string/string.c:302
	s is NULL
[00:00:27.840,000] <err> os: r0/a1:  0x00000004  r1/a2:  0x0000012e  r2/a3:  0x00000040
[00:00:27.840,000] <err> os: r3/a4:  0x08004809 r12/ip:  0x00000000 r14/lr:  0x08005fd9
[00:00:27.840,000] <err> os:  xpsr:  0x61000000
[00:00:27.840,000] <err> os: Faulting instruction address (r15/pc): 0x08012fa6
[00:00:27.840,000] <err> os: >>> ZEPHYR FATAL ERROR 4: Kernel panic on CPU 0
[00:00:27.840,000] <err> os: Current thread: 0x20041138 (unknown)
[00:00:27.885,000] <err> os: Halting system

Environment (please complete the following information):

• OS: Kubuntu 20.04
• Toolchain: Zephyr SDK 0.11.4, west 0.7.3
• Tag v2.4.0

[KozhinovAlexander]

@armandciejak Please provide your changes to string.c in terms of added asserts: you can put changed string.c here or better create new PR as a draft with your changes.
Are you facing same problem with other samples using same memcpy function from string.c?

[armandciejak]

Here is the patch for memcpy:

diff --git a/lib/libc/minimal/source/string/string.c b/lib/libc/minimal/source/string/string.c
index 9344f824ca..3fb9a06933 100644
--- a/lib/libc/minimal/source/string/string.c
+++ b/lib/libc/minimal/source/string/string.c
@@ -9,6 +9,7 @@
 #include <string.h>
 #include <stdint.h>
 #include <sys/types.h>
+#include <sys/__assert.h>
 
 /**
  *
@@ -297,6 +298,9 @@ void *memmove(void *d, const void *s, size_t n)
 
 void *memcpy(void *_MLIBC_RESTRICT d, const void *_MLIBC_RESTRICT s, size_t n)
 {
+       __ASSERT(d != NULL, "d is NULL");
+       __ASSERT(s != NULL, "s is NULL");
+
        /* attempt word-sized copying only if buffers have identical alignment */
 
        unsigned char *d_byte = (unsigned char *)d;

I tested the samples/net/sockets/dumb_http_server application without problems.

[jukkar]

Here is the patch for memcpy:

Could you send a PR for the memcpy() asserts, they look useful to have.

[KozhinovAlexander]

Here is the patch for memcpy:

Could you send a PR for the memcpy() asserts, they look useful to have.

Does this asserts fire only in debug mode or always? I would prefer them only for debugging purposes.

[jukkar]

Does this asserts fire only in debug mode or always? I would prefer them only for debugging purposes.

Asserts are off by default.

[jukkar]

@armandciejak fix proposal at #28755. I tested it but would appreciate if you can verify it works ok for you too.

[armandciejak]

@jukkar Thanks for the fix. I tested it successfully (zsock_getsockname works now fine).

Unfortunately I get another assert after a few round of curl downloading the home page: while true; do curl http://192.168.1.241:8080; done.

192.168.1.170 - "GET / HTTP/1.1" 200
192.168.1.170 - "GET / HTTP/1.1" 200
192.168.1.170 - "GET / HTTP/1.1" 200
192.168.1.170 - "GET / HTTP/1.1" 200
ASSERTION FAIL [conn->in_retransmission == 1] @ WEST_TOPDIR/zephyr/subsys/net/ip/tcp2.c:427
	Not in retransmission
[00:00:43.248,000] <err> os: r0/a1:  0x00000004  r1/a2:  0x000001ab  r2/a3:  0x00000040
[00:00:43.248,000] <err> os: r3/a4:  0x08004809 r12/ip:  0x00000000 r14/lr:  0x08009abf
[00:00:43.248,000] <err> os:  xpsr:  0x61000000
[00:00:43.248,000] <err> os: Faulting instruction address (r15/pc): 0x08012fca
[00:00:43.248,000] <err> os: >>> ZEPHYR FATAL ERROR 4: Kernel panic on CPU 0
[00:00:43.248,000] <err> os: Current thread: 0x20041568 (unknown)
[00:00:43.271,000] <err> os: Halting system

[jukkar]

Unfortunately I get another assert after a few round of curl downloading the home page: while true; do curl http://192.168.1.241:8080; done.

Ok, that assert seems to be unnecessary. I added a commit to the #28755 that does not assert in this case.

[armandciejak]

I created a new issue for the "Not in retransmission" assert: #28758.