On Sun, Jun 07, 2026 at 08:50:26PM -0400, Jon Maloy wrote:

udp_peek_addr() initialises struct msghdr without setting msg_iov, leaving it implicitly NULL. Coverity flags this as FORWARD_NULL, believing recvmsg() will dereference the NULL pointer.

In practice, msg_iovlen being zero means the kernel never touches msg_iov, so the warning is a false positive. We now provide a one-byte dummy iov to make msg_iov non-NULL, hence suppressing this warning without changing the function's behaviour.

Signed-off-by: Jon Maloy

Following on from our discussion yesterday. Although this is based on my suggestion, I'm now inclined to believe v1 is marginally less ugly. Fwiw, I double checked the code and can now confirm that this is only called for packets arriving on a "listening" socket, rather than a flow-specific socket. Typically that will only be once, or at worst a handful of times, per flow.

---- v2: - Make the dummy iov conditional on an ANALYZER macro, so it has zero runtime cost in production builds. - Add a new 'analyzer' Makefile target (similar to 'valgrind') that defines ANALYZER via CPPFLAGS for use with static analysis builds. --- Makefile | 3 +++ udp.c | 11 +++++++++++ 2 files changed, 14 insertions(+)

diff --git a/Makefile b/Makefile index 0a0a60b0..4dcf4cd1 100644 --- a/Makefile +++ b/Makefile @@ -122,6 +122,9 @@ passt-repair: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h pesto: BASE_CPPFLAGS += -DPESTO pesto: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h

+analyzer: BASE_CPPFLAGS += -DANALYZER +analyzer: all + valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ rt_sigreturn getpid gettid kill clock_gettime \ mmap|mmap2 munmap open unlink gettimeofday futex \ diff --git a/udp.c b/udp.c index c28d6ee2..36c8c070 100644 --- a/udp.c +++ b/udp.c @@ -734,9 +734,20 @@ static int udp_peek_addr(int s, union sockaddr_inany *src, { char sastr[SOCKADDR_STRLEN], dstr[INANY_ADDRSTRLEN]; char cmsg[PKTINFO_SPACE]; +#ifdef ANALYZER + char dummy; + struct iovec iov = { + .iov_base = &dummy, + .iov_len = sizeof(dummy), + }; +#endif /* ANALYZER */ struct msghdr msg = { .msg_name = src, .msg_namelen = sizeof(*src), +#ifdef ANALYZER + .msg_iov = &iov, + .msg_iovlen = 1, +#endif /* ANALYZER */ .msg_control = cmsg, .msg_controllen = sizeof(cmsg), }; -- 2.52.0

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson

On Fri, Jun 12, 2026 at 01:05:25AM +0200, Stefano Brivio wrote:

On Sun, 7 Jun 2026 20:50:26 -0400 Jon Maloy wrote:

...

udp_peek_addr() initialises struct msghdr without setting msg_iov, leaving it implicitly NULL. Coverity flags this as FORWARD_NULL, believing recvmsg() will dereference the NULL pointer.

In practice, msg_iovlen being zero means the kernel never touches msg_iov, so the warning is a false positive. We now provide a one-byte dummy iov to make msg_iov non-NULL, hence suppressing this warning without changing the function's behaviour.

Signed-off-by: Jon Maloy

---- v2: - Make the dummy iov conditional on an ANALYZER macro, so it has zero runtime cost in production builds.

As I mentioned offline, I'd rather go with v1, because a macro that's not enabled / seen by any static checker isn't really convenient. And I'll take care of reporting this to Coverity.

Just one detail, regardless of that:

...

- Add a new 'analyzer' Makefile target (similar to 'valgrind') that defines ANALYZER via CPPFLAGS for use with static analysis builds. --- Makefile | 3 +++ udp.c | 11 +++++++++++ 2 files changed, 14 insertions(+)

diff --git a/Makefile b/Makefile index 0a0a60b0..4dcf4cd1 100644 --- a/Makefile +++ b/Makefile @@ -122,6 +122,9 @@ passt-repair: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h pesto: BASE_CPPFLAGS += -DPESTO pesto: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h

+analyzer: BASE_CPPFLAGS += -DANALYZER +analyzer: all + valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ rt_sigreturn getpid gettid kill clock_gettime \ mmap|mmap2 munmap open unlink gettimeofday futex \ diff --git a/udp.c b/udp.c index c28d6ee2..36c8c070 100644 --- a/udp.c +++ b/udp.c @@ -734,9 +734,20 @@ static int udp_peek_addr(int s, union sockaddr_inany *src, { char sastr[SOCKADDR_STRLEN], dstr[INANY_ADDRSTRLEN]; char cmsg[PKTINFO_SPACE]; +#ifdef ANALYZER + char dummy; + struct iovec iov = { + .iov_base = &dummy, + .iov_len = sizeof(dummy), + }; +#endif /* ANALYZER */ struct msghdr msg = { .msg_name = src, .msg_namelen = sizeof(*src), +#ifdef ANALYZER + .msg_iov = &iov, + .msg_iovlen = 1,

...can't we pass 0 instead? What happens? That looks like less potential for overhead in the sense of a possible copy_to_user() that we don't want to trigger.

Yeah, that would be interesting to know. Furthermore, if we pass zero along with a valid-but-arbitary pointer, that might be enough to convince the analyzer. &msg could be a suitable pointer (with an explanatory comment) - weird but harmless.

...

+#endif /* ANALYZER */ .msg_control = cmsg, .msg_controllen = sizeof(cmsg), };

-- Stefano

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson