Skip to contentSkip to conversation

ipv6: Fix may be used uninitialized warning in rt6_check

PATCH Applied Steffen Klassert 3 participants 5 messages Aug 25, 2017 – Aug 26, 2017

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Steffen Klassert

author
#0 lore raw

PATCH 1/1 ipv6: Fix may be used uninitialized warning in rt6_check

rt_cookie might be used uninitialized, fix this by initializing it.

Fixes c5cff8561d2d ("ipv6: add rcu grace period before freeing fib6_node")
Signed-off-by Steffen Klassert <[email protected]>
---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv6/route.c +1 -1
index a9d3564..48c8c92 100644
@@ -1289,7 +1289,7 @@ static void rt6_dst_from_metrics_check(struct rt6_info *rt)
 
 static struct dst_entry *rt6_check(struct rt6_info *rt, u32 cookie)
 {
-	u32 rt_cookie;
+	u32 rt_cookie = 0;
 
 	if (!rt6_get_cookie_safe(rt, &rt_cookie) || rt_cookie != cookie)
 		return NULL;
Signature · 1 line
2.7.4
author
#1 lore raw
Quoted 14 lines · Steffen Klassert

rt_cookie might be used uninitialized, fix this by initializing it.

Fixes c5cff8561d2d ("ipv6: add rcu grace period before freeing fib6_node")
Signed-off-by Steffen Klassert <[email protected]>
---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv6/route.c +0 -0
index a9d3564..48c8c92 100644

net/ipv6/route.c static void rt6_dst_from_metrics_check(struct rt6_info *rt) · line 1295

Quoted 8 lines
net/ipv6/route.c +1 -1
@@ -1289,7 +1289,7 @@ static void rt6_dst_from_metrics_check(struct rt6_info *rt)
 
 static struct dst_entry *rt6_check(struct rt6_info *rt, u32 cookie)
 {
-	u32 rt_cookie;
+	u32 rt_cookie = 0;
 
 	if (!rt6_get_cookie_safe(rt, &rt_cookie) || rt_cookie != cookie)
 		return NULL;

The compiler warning seems to be a false positive, as rt_cookie != cookie is only checked if rt6_get_cookie_safe returns true in which case rt_cookie is initialized.

Please disregard this patch.

#2 lore raw
Quoted 21 lines · On Fri, 25 Aug 2017 09:52:17 +0200 Steffen Klassert…
Quoted 20 lines · Steffen Klassert

rt_cookie might be used uninitialized, fix this by initializing it.

Fixes c5cff8561d2d ("ipv6: add rcu grace period before freeing fib6_node")
Signed-off-by Steffen Klassert <[email protected]>
---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv6/route.c +1 -1
index a9d3564..48c8c92 100644
@@ -1289,7 +1289,7 @@ static void rt6_dst_from_metrics_check(struct rt6_info *rt)
 
 static struct dst_entry *rt6_check(struct rt6_info *rt, u32 cookie)
 {
-	u32 rt_cookie;
+	u32 rt_cookie = 0;
Quoted 8 lines

net/ipv6/route.c static void rt6_dst_from_metrics_check(struct rt6_info *rt) · line 1295

Quoted 2 lines
net/ipv6/route.c +0 -0
 	if (!rt6_get_cookie_safe(rt, &rt_cookie) || rt_cookie != cookie)
 		return NULL;

The compiler warning seems to be a false positive, as rt_cookie != cookie is only checked if rt6_get_cookie_safe returns true in which case rt_cookie is initialized.

Please disregard this patch.

...or not? I was thinking of sending a similar patch with uninitialized_var(rt_cookie), but it seems we have similar cases where we just initialize to zero instead.

I wonder which approach is considered the most acceptable nowadays. I would be in favour of uninitialized_var() as it doesn't change the binary output, but https://lwn.net/Articles/529954/ also contains some valid criticism. Ideas?

maintainer
#3 lore raw

From: Stefano Brivio <[email protected]>
Date: Fri, 25 Aug 2017 11:02:06 +0200

Quoted 33 lines
Quoted 21 lines · On Fri, 25 Aug 2017 09:52:17 +0200 Steffen Klassert…
Quoted 20 lines · Steffen Klassert

rt_cookie might be used uninitialized, fix this by initializing it.

Fixes c5cff8561d2d ("ipv6: add rcu grace period before freeing fib6_node")
Signed-off-by Steffen Klassert <[email protected]>
---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/ipv6/route.c +1 -1
index a9d3564..48c8c92 100644
@@ -1289,7 +1289,7 @@ static void rt6_dst_from_metrics_check(struct rt6_info *rt)
 
 static struct dst_entry *rt6_check(struct rt6_info *rt, u32 cookie)
 {
-	u32 rt_cookie;
+	u32 rt_cookie = 0;
Quoted 8 lines

net/ipv6/route.c static void rt6_dst_from_metrics_check(struct rt6_info *rt) · line 1295

Quoted 2 lines
net/ipv6/route.c +0 -0
 	if (!rt6_get_cookie_safe(rt, &rt_cookie) || rt_cookie != cookie)
 		return NULL;

The compiler warning seems to be a false positive, as rt_cookie != cookie is only checked if rt6_get_cookie_safe returns true in which case rt_cookie is initialized.

Please disregard this patch.

Quoted 8 lines

...or not? I was thinking of sending a similar patch with uninitialized_var(rt_cookie), but it seems we have similar cases where we just initialize to zero instead.

I wonder which approach is considered the most acceptable nowadays. I would be in favour of uninitialized_var() as it doesn't change the binary output, but https://lwn.net/Articles/529954/ also contains some valid criticism. Ideas?

Generally speaking I guess initializing to zero is Ok to do.

As far as which approach is better, I don't have any strong opinion.

So I will probably just apply Steffen's patch.