Re: [PATCH v2] udp: Provide dummy iov in udp_peek_addr() to avoid Coverity warning

[David Gibson <david@gibson.dropbear.id.au>]

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

On Fri, Jun 12, 2026 at 01:05:25AM +0200, Stefano Brivio wrote:
> On Sun,  7 Jun 2026 20:50:26 -0400
> Jon Maloy <jmaloy@redhat.com> 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>
> > 
> > ----
> > 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

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