LinuxLists.cc - [PATCH] flow_dissector: Pre-initialize ip_proto in __skb_flow

2015-06-25 13:10:44

Subject: [PATCH] flow_dissector: Pre-initialize ip_proto in __skb_flow_dissect()

net/core/flow_dissector.c: In function ‘__skb_flow_dissect’:
net/core/flow_dissector.c:132: warning: ‘ip_proto’ may be used uninitialized in this function

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
This may be a false positive, but the state machine in
__skb_flow_dissect() is a bit hard to follow.
As I believe it is controlled by a packet received from the network, the
only safe thing to do is to pre-initialize ip_proto.
---
net/core/flow_dissector.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 476e5dda59e19822..2a834c6179b9973e 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -129,7 +129,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
struct flow_dissector_key_ports *key_ports;
struct flow_dissector_key_tags *key_tags;
struct flow_dissector_key_keyid *key_keyid;
- u8 ip_proto;
+ u8 ip_proto = 0;

if (!data) {
data = skb->data;
--
1.9.1

2015-06-25 13:21:48

by David Miller

[permalink] [raw]

Subject: Re: [PATCH] flow_dissector: Pre-initialize ip_proto in __skb_flow_dissect()

From: Geert Uytterhoeven <[email protected]>
Date: Thu, 25 Jun 2015 15:10:32 +0200

> net/core/flow_dissector.c: In function ?__skb_flow_dissect?:
> net/core/flow_dissector.c:132: warning: ?ip_proto? may be used uninitialized in this function
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> This may be a false positive, but the state machine in
> __skb_flow_dissect() is a bit hard to follow.
> As I believe it is controlled by a packet received from the network, the
> only safe thing to do is to pre-initialize ip_proto.

Actually I think this is a real bug, because for the ETH_P_MPLS_* cases I cannot
see what will always set ip_proto before it gets used as an input.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2015-06-25 13:48:51

by Jiri Pirko

[permalink] [raw]

Subject: Re: [PATCH] flow_dissector: Pre-initialize ip_proto in __skb_flow_dissect()

Thu, Jun 25, 2015 at 03:33:31PM CEST, [email protected] wrote:
>From: Geert Uytterhoeven <[email protected]>
>Date: Thu, 25 Jun 2015 15:10:32 +0200
>
>> net/core/flow_dissector.c: In function ‘__skb_flow_dissect’:
>> net/core/flow_dissector.c:132: warning: ‘ip_proto’ may be used uninitialized in this function
>>
>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>> ---
>> This may be a false positive, but the state machine in
>> __skb_flow_dissect() is a bit hard to follow.
>> As I believe it is controlled by a packet received from the network, the
>> only safe thing to do is to pre-initialize ip_proto.
>
>Actually I think this is a real bug, because for the ETH_P_MPLS_* cases I cannot
>see what will always set ip_proto before it gets used as an input.

I think that MPLS cases are ok. In this case, return is always hit.
I believe this is false positive.

2015-06-25 16:07:53

by Tom Herbert

[permalink] [raw]

Subject: Re: [PATCH] flow_dissector: Pre-initialize ip_proto in __skb_flow_dissect()

On Thu, Jun 25, 2015 at 6:10 AM, Geert Uytterhoeven
<[email protected]> wrote:
> net/core/flow_dissector.c: In function ‘__skb_flow_dissect’:
> net/core/flow_dissector.c:132: warning: ‘ip_proto’ may be used uninitialized in this function
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> This may be a false positive, but the state machine in
> __skb_flow_dissect() is a bit hard to follow.
> As I believe it is controlled by a packet received from the network, the
> only safe thing to do is to pre-initialize ip_proto.
> ---
> net/core/flow_dissector.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 476e5dda59e19822..2a834c6179b9973e 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -129,7 +129,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
> struct flow_dissector_key_ports *key_ports;
> struct flow_dissector_key_tags *key_tags;
> struct flow_dissector_key_keyid *key_keyid;
> - u8 ip_proto;
> + u8 ip_proto = 0;
>
> if (!data) {
> data = skb->data;
> --
> 1.9.1
>

Acked-by: Tom Herbert <[email protected]>

2015-06-28 23:54:29

by David Miller

[permalink] [raw]