[PATCH] [net-next] net: sched: avoid uninitialized variable use

[PATCH] [net-next] net: sched: avoid uninitialized variable use

[Arnd Bergmann]

gcc has identified a code path in which we pass uninitialized
data into tc_dump_tfilter():

net/sched/cls_api.c: In function 'tc_dump_tfilter':
net/sched/cls_api.c:1268:8: error: 'parent' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This initializes the variable to the value it had before the previous
change.

Fixes: 7960d1daf278 ("net: sched: use block index as a handle instead of qdisc when block is shared")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
----
I don't know if my patch is the best way to address the issue, but
if not, then at least it helps show what the warning is about
and lets someone else come up with a better solution.
---
 net/sched/cls_api.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index e500d11da9cd..a95bfc8fc442 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1317,6 +1317,7 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 		block = tcf_block_lookup(net, tcm->tcm_block_index);
 		if (!block)
 			goto out;
+		parent = tcm->tcm_parent;
 	} else {
 		const struct Qdisc_class_ops *cops;
 		struct net_device *dev;
-- 
2.9.0

Re: [PATCH] [net-next] net: sched: avoid uninitialized variable use

[Jiri Pirko]

Thu, Jan 18, 2018 at 02:17:28PM CET, arnd@arndb.de wrote:
>gcc has identified a code path in which we pass uninitialized
>data into tc_dump_tfilter():
>
>net/sched/cls_api.c: In function 'tc_dump_tfilter':
>net/sched/cls_api.c:1268:8: error: 'parent' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
>This initializes the variable to the value it had before the previous
>change.
>
>Fixes: 7960d1daf278 ("net: sched: use block index as a handle instead of qdisc when block is shared")
>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>----
>I don't know if my patch is the best way to address the issue, but
>if not, then at least it helps show what the warning is about
>and lets someone else come up with a better solution.

I already sent a fix for this:
http://patchwork.ozlabs.org/patch/862787/

Re: [PATCH] [net-next] net: sched: avoid uninitialized variable use

[Arnd Bergmann]

On Thu, Jan 18, 2018 at 2:49 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Jan 18, 2018 at 02:17:28PM CET, arnd@arndb.de wrote:
>>gcc has identified a code path in which we pass uninitialized
>>data into tc_dump_tfilter():
>>
>>net/sched/cls_api.c: In function 'tc_dump_tfilter':
>>net/sched/cls_api.c:1268:8: error: 'parent' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>
>>This initializes the variable to the value it had before the previous
>>change.
>>
>>Fixes: 7960d1daf278 ("net: sched: use block index as a handle instead of qdisc when block is shared")
>>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>----
>>I don't know if my patch is the best way to address the issue, but
>>if not, then at least it helps show what the warning is about
>>and lets someone else come up with a better solution.
>
> I already sent a fix for this:
> http://patchwork.ozlabs.org/patch/862787/

Ok. I've looked at your patch for way too long now and still don't see how
you've shown it to be correct. Shouldn't there be a at least a comment
to explain why zero is an appropriate initialization value in that case?

      Arnd

Re: [PATCH] [net-next] net: sched: avoid uninitialized variable use

[Jiri Pirko]

Thu, Jan 18, 2018 at 03:19:14PM CET, arnd@arndb.de wrote:
>On Thu, Jan 18, 2018 at 2:49 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Thu, Jan 18, 2018 at 02:17:28PM CET, arnd@arndb.de wrote:
>>>gcc has identified a code path in which we pass uninitialized
>>>data into tc_dump_tfilter():
>>>
>>>net/sched/cls_api.c: In function 'tc_dump_tfilter':
>>>net/sched/cls_api.c:1268:8: error: 'parent' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>>
>>>This initializes the variable to the value it had before the previous
>>>change.
>>>
>>>Fixes: 7960d1daf278 ("net: sched: use block index as a handle instead of qdisc when block is shared")
>>>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>>----
>>>I don't know if my patch is the best way to address the issue, but
>>>if not, then at least it helps show what the warning is about
>>>and lets someone else come up with a better solution.
>>
>> I already sent a fix for this:
>> http://patchwork.ozlabs.org/patch/862787/
>
>Ok. I've looked at your patch for way too long now and still don't see how
>you've shown it to be correct. Shouldn't there be a at least a comment
>to explain why zero is an appropriate initialization value in that case?

Okay. Will add comment.


>
>      Arnd