public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH v2] udp: Provide dummy iov in udp_peek_addr() to avoid Coverity warning
@ 2026-06-08  0:50 Jon Maloy
  2026-06-09  0:42 ` David Gibson
  0 siblings, 1 reply; 2+ messages in thread
From: Jon Maloy @ 2026-06-08  0:50 UTC (permalink / raw)
  To: sbrivio, david, jmaloy, passt-dev

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 <jmaloy@redhat.com>

----
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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] udp: Provide dummy iov in udp_peek_addr() to avoid Coverity warning
  2026-06-08  0:50 [PATCH v2] udp: Provide dummy iov in udp_peek_addr() to avoid Coverity warning Jon Maloy
@ 2026-06-09  0:42 ` David Gibson
  0 siblings, 0 replies; 2+ messages in thread
From: David Gibson @ 2026-06-09  0:42 UTC (permalink / raw)
  To: Jon Maloy; +Cc: sbrivio, passt-dev

[-- Attachment #1: Type: text/plain, Size: 2827 bytes --]

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 <jmaloy@redhat.com>

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-09  1:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-08  0:50 [PATCH v2] udp: Provide dummy iov in udp_peek_addr() to avoid Coverity warning Jon Maloy
2026-06-09  0:42 ` David Gibson

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).