Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
route delete ifp NULL check
To:
tech@openbsd.org
Date:
Sun, 3 Aug 2025 14:26:20 +0200

Download raw body.

Thread
Hi,

Coverity complains that we check ifp NULL pointer in rtrequest_delete(),
but later on we dereference it unconditinally.

The only caller that might have a NULL in ifp is nd6_free().  But
there we also dereference it later.  I think the exclusive net lock
protects us there, but I want to assure it with a kassert.

When we unlock ND6, we have to reconsider this.

ok?

bluhm

Index: net/route.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
diff -u -p -r1.447 route.c
--- net/route.c	14 Jul 2025 08:48:51 -0000	1.447
+++ net/route.c	3 Aug 2025 12:15:08 -0000
@@ -907,7 +907,7 @@ rtrequest_delete(struct rt_addrinfo *inf
 		return (ESRCH);
 
 	/* Make sure that's the route the caller want to delete. */
-	if (ifp != NULL && ifp->if_index != rt->rt_ifidx) {
+	if (ifp->if_index != rt->rt_ifidx) {
 		rtfree(rt);
 		return (ESRCH);
 	}
Index: netinet6/nd6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v
diff -u -p -r1.295 nd6.c
--- netinet6/nd6.c	3 Aug 2025 04:11:57 -0000	1.295
+++ netinet6/nd6.c	3 Aug 2025 12:17:29 -0000
@@ -647,6 +647,7 @@ nd6_free(struct rtentry *rt, int i_am_ro
 	NET_ASSERT_LOCKED_EXCLUSIVE();
 
 	ifp = if_get(rt->rt_ifidx);
+	KASSERT(ifp != NULL);
 
 	if (!i_am_router) {
 		if (ln->ln_router) {