[PATCH] networking: fix potential deref-of-null

Adam Goldman adamg at pobox.com
Mon Mar 17 08:50:41 UTC 2025


On Tue, Jan 21, 2025 at 08:58:57PM +0300, Maks Mishin wrote:
> The initial condition with the OR operator does not guarantee 
> that the pointer ci will be non-zero when dereferencing, 
> for example, in iproute.c:314: `if (ci->rta_expires)`.
> For fix this, the OR operator is replaced by the AND operator.
 
> -		if ((r->rtm_flags & RTM_F_CLONED) || (ci && ci->rta_expires)) {
> +		if ((r->rtm_flags & RTM_F_CLONED) && (ci && ci->rta_expires)) {

Is it valid for RTM_F_CLONED to be set but ci to be NULL? If so, this 
changes the semantics: "cache" will no longer be printed if RTM_F_CLONED 
is set and ci is NULL.

A change like the following would preserve original semantics:

--- iproute.c.orig	2024-04-16 00:23:50.000000000 -0700
+++ iproute.c	2025-03-17 01:47:13.000000000 -0700
@@ -307,17 +307,13 @@
 		if (tb[RTA_CACHEINFO]) {
 			ci = RTA_DATA(tb[RTA_CACHEINFO]);
 		}
-		if ((r->rtm_flags & RTM_F_CLONED) || (ci && ci->rta_expires)) {
-			if (r->rtm_flags & RTM_F_CLONED) {
-				printf("%c    cache ", _SL_);
-			}
+		if (r->rtm_flags & RTM_F_CLONED) {
+			printf("%c    cache ", _SL_);
+		}
+		if (ci) {
 			if (ci->rta_expires) {
 				printf(" expires %dsec", ci->rta_expires / get_hz());
 			}
-			if (ci->rta_error != 0) {
-				printf(" error %d", ci->rta_error);
-			}
-		} else if (ci) {
 			if (ci->rta_error != 0)
 				printf(" error %d", ci->rta_error);
 		}



More information about the busybox mailing list