* Re: [Bridge] bridge: per vlan dst_metadata netlink support
@ 2017-02-07 11:25 ` Colin Ian King
0 siblings, 0 replies; 10+ messages in thread
From: Colin Ian King @ 2017-02-07 11:25 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Nikolay Aleksandrov, netdev@vger.kernel.org, bridge,
David S. Miller
Hi,
Static analysis with CoverityScan on net/bridge/br_netlink_tunnel.c has
picked up and issue where tb[] is being referenced but contains
uninitialized data.
222 int br_parse_vlan_tunnel_info(struct nlattr *attr,
223 struct vtunnel_info *tinfo)
224 {
1. var_decl: Declaring variable tb without initializer.
225 struct nlattr *tb[IFLA_BRIDGE_VLAN_TUNNEL_MAX + 1];
226 u32 tun_id;
227 u16 vid, flags = 0;
228 int err;
229
230 memset(tinfo, 0, sizeof(*tinfo));
231
CID 1400055 (#1 of 1): Uninitialized pointer read (UNINIT)2.
uninit_use: Using uninitialized value tb[IFLA_BRIDGE_VLAN_TUNNEL_ID].
232 if (!tb[IFLA_BRIDGE_VLAN_TUNNEL_ID] ||
233 !tb[IFLA_BRIDGE_VLAN_TUNNEL_VID])
234 return -EINVAL;
Colin
^ permalink raw reply [flat|nested] 10+ messages in thread* RE: bridge: per vlan dst_metadata netlink support
@ 2017-02-07 11:25 ` Colin Ian King
0 siblings, 0 replies; 10+ messages in thread
From: Colin Ian King @ 2017-02-07 11:25 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Nikolay Aleksandrov, David S. Miller, bridge,
netdev@vger.kernel.org
Hi,
Static analysis with CoverityScan on net/bridge/br_netlink_tunnel.c has
picked up and issue where tb[] is being referenced but contains
uninitialized data.
222 int br_parse_vlan_tunnel_info(struct nlattr *attr,
223 struct vtunnel_info *tinfo)
224 {
1. var_decl: Declaring variable tb without initializer.
225 struct nlattr *tb[IFLA_BRIDGE_VLAN_TUNNEL_MAX + 1];
226 u32 tun_id;
227 u16 vid, flags = 0;
228 int err;
229
230 memset(tinfo, 0, sizeof(*tinfo));
231
CID 1400055 (#1 of 1): Uninitialized pointer read (UNINIT)2.
uninit_use: Using uninitialized value tb[IFLA_BRIDGE_VLAN_TUNNEL_ID].
232 if (!tb[IFLA_BRIDGE_VLAN_TUNNEL_ID] ||
233 !tb[IFLA_BRIDGE_VLAN_TUNNEL_VID])
234 return -EINVAL;
Colin
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Bridge] bridge: per vlan dst_metadata netlink support
2017-02-07 11:25 ` Colin Ian King
@ 2017-02-07 11:29 ` Nikolay Aleksandrov via Bridge
-1 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2017-02-07 11:29 UTC (permalink / raw)
To: Colin Ian King, Roopa Prabhu
Cc: netdev@vger.kernel.org, bridge, David S. Miller
On 07/02/17 12:25, Colin Ian King wrote:
> Hi,
>
> Static analysis with CoverityScan on net/bridge/br_netlink_tunnel.c has
> picked up and issue where tb[] is being referenced but contains
> uninitialized data.
>
> 222 int br_parse_vlan_tunnel_info(struct nlattr *attr,
> 223 struct vtunnel_info *tinfo)
> 224 {
>
> 1. var_decl: Declaring variable tb without initializer.
>
> 225 struct nlattr *tb[IFLA_BRIDGE_VLAN_TUNNEL_MAX + 1];
> 226 u32 tun_id;
> 227 u16 vid, flags = 0;
> 228 int err;
> 229
> 230 memset(tinfo, 0, sizeof(*tinfo));
> 231
>
> CID 1400055 (#1 of 1): Uninitialized pointer read (UNINIT)2.
> uninit_use: Using uninitialized value tb[IFLA_BRIDGE_VLAN_TUNNEL_ID].
>
> 232 if (!tb[IFLA_BRIDGE_VLAN_TUNNEL_ID] ||
> 233 !tb[IFLA_BRIDGE_VLAN_TUNNEL_VID])
> 234 return -EINVAL;
>
> Colin
>
Good catch, these should go after the parsing.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: bridge: per vlan dst_metadata netlink support
@ 2017-02-07 11:29 ` Nikolay Aleksandrov via Bridge
0 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov via Bridge @ 2017-02-07 11:29 UTC (permalink / raw)
To: Colin Ian King, Roopa Prabhu
Cc: netdev@vger.kernel.org, bridge, David S. Miller
On 07/02/17 12:25, Colin Ian King wrote:
> Hi,
>
> Static analysis with CoverityScan on net/bridge/br_netlink_tunnel.c has
> picked up and issue where tb[] is being referenced but contains
> uninitialized data.
>
> 222 int br_parse_vlan_tunnel_info(struct nlattr *attr,
> 223 struct vtunnel_info *tinfo)
> 224 {
>
> 1. var_decl: Declaring variable tb without initializer.
>
> 225 struct nlattr *tb[IFLA_BRIDGE_VLAN_TUNNEL_MAX + 1];
> 226 u32 tun_id;
> 227 u16 vid, flags = 0;
> 228 int err;
> 229
> 230 memset(tinfo, 0, sizeof(*tinfo));
> 231
>
> CID 1400055 (#1 of 1): Uninitialized pointer read (UNINIT)2.
> uninit_use: Using uninitialized value tb[IFLA_BRIDGE_VLAN_TUNNEL_ID].
>
> 232 if (!tb[IFLA_BRIDGE_VLAN_TUNNEL_ID] ||
> 233 !tb[IFLA_BRIDGE_VLAN_TUNNEL_VID])
> 234 return -EINVAL;
>
> Colin
>
Good catch, these should go after the parsing.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bridge] [PATCH net-next] bridge: tunnel: fix attribute checks in br_parse_vlan_tunnel_info
2017-02-07 11:29 ` Nikolay Aleksandrov via Bridge
@ 2017-02-07 11:46 ` Nikolay Aleksandrov
-1 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2017-02-07 11:46 UTC (permalink / raw)
To: netdev; +Cc: Nikolay Aleksandrov, Colin Ian King, roopa, bridge,
David S. Miller
These checks should go after the attributes have been parsed otherwise
we're using tb uninitialized.
Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
Reported-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/bridge/br_netlink_tunnel.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/bridge/br_netlink_tunnel.c b/net/bridge/br_netlink_tunnel.c
index 99c68012c9d4..62eaf750bd9e 100644
--- a/net/bridge/br_netlink_tunnel.c
+++ b/net/bridge/br_netlink_tunnel.c
@@ -229,15 +229,15 @@ int br_parse_vlan_tunnel_info(struct nlattr *attr,
memset(tinfo, 0, sizeof(*tinfo));
- if (!tb[IFLA_BRIDGE_VLAN_TUNNEL_ID] ||
- !tb[IFLA_BRIDGE_VLAN_TUNNEL_VID])
- return -EINVAL;
-
err = nla_parse_nested(tb, IFLA_BRIDGE_VLAN_TUNNEL_MAX,
attr, vlan_tunnel_policy);
if (err < 0)
return err;
+ if (!tb[IFLA_BRIDGE_VLAN_TUNNEL_ID] ||
+ !tb[IFLA_BRIDGE_VLAN_TUNNEL_VID])
+ return -EINVAL;
+
tun_id = nla_get_u32(tb[IFLA_BRIDGE_VLAN_TUNNEL_ID]);
vid = nla_get_u16(tb[IFLA_BRIDGE_VLAN_TUNNEL_VID]);
if (vid >= VLAN_VID_MASK)
--
2.1.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next] bridge: tunnel: fix attribute checks in br_parse_vlan_tunnel_info
@ 2017-02-07 11:46 ` Nikolay Aleksandrov
0 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2017-02-07 11:46 UTC (permalink / raw)
To: netdev; +Cc: roopa, bridge, Colin Ian King, David S. Miller,
Nikolay Aleksandrov
These checks should go after the attributes have been parsed otherwise
we're using tb uninitialized.
Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
Reported-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/bridge/br_netlink_tunnel.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/bridge/br_netlink_tunnel.c b/net/bridge/br_netlink_tunnel.c
index 99c68012c9d4..62eaf750bd9e 100644
--- a/net/bridge/br_netlink_tunnel.c
+++ b/net/bridge/br_netlink_tunnel.c
@@ -229,15 +229,15 @@ int br_parse_vlan_tunnel_info(struct nlattr *attr,
memset(tinfo, 0, sizeof(*tinfo));
- if (!tb[IFLA_BRIDGE_VLAN_TUNNEL_ID] ||
- !tb[IFLA_BRIDGE_VLAN_TUNNEL_VID])
- return -EINVAL;
-
err = nla_parse_nested(tb, IFLA_BRIDGE_VLAN_TUNNEL_MAX,
attr, vlan_tunnel_policy);
if (err < 0)
return err;
+ if (!tb[IFLA_BRIDGE_VLAN_TUNNEL_ID] ||
+ !tb[IFLA_BRIDGE_VLAN_TUNNEL_VID])
+ return -EINVAL;
+
tun_id = nla_get_u32(tb[IFLA_BRIDGE_VLAN_TUNNEL_ID]);
vid = nla_get_u16(tb[IFLA_BRIDGE_VLAN_TUNNEL_VID]);
if (vid >= VLAN_VID_MASK)
--
2.1.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Bridge] [PATCH net-next] bridge: tunnel: fix attribute checks in br_parse_vlan_tunnel_info
2017-02-07 11:46 ` Nikolay Aleksandrov
@ 2017-02-07 13:24 ` Roopa Prabhu
-1 siblings, 0 replies; 10+ messages in thread
From: Roopa Prabhu @ 2017-02-07 13:24 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, bridge, David S. Miller, Colin Ian King
On 2/7/17, 3:46 AM, Nikolay Aleksandrov wrote:
> These checks should go after the attributes have been parsed otherwise
> we're using tb uninitialized.
>
> Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
> Reported-by: Colin Ian King <colin.king@canonical.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
>
ouch, am surprised i did not catch this. Thanks Colin and Nikolay.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next] bridge: tunnel: fix attribute checks in br_parse_vlan_tunnel_info
@ 2017-02-07 13:24 ` Roopa Prabhu
0 siblings, 0 replies; 10+ messages in thread
From: Roopa Prabhu @ 2017-02-07 13:24 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, bridge, Colin Ian King, David S. Miller
On 2/7/17, 3:46 AM, Nikolay Aleksandrov wrote:
> These checks should go after the attributes have been parsed otherwise
> we're using tb uninitialized.
>
> Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
> Reported-by: Colin Ian King <colin.king@canonical.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
>
ouch, am surprised i did not catch this. Thanks Colin and Nikolay.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Bridge] [PATCH net-next] bridge: tunnel: fix attribute checks in br_parse_vlan_tunnel_info
2017-02-07 11:46 ` Nikolay Aleksandrov
@ 2017-02-07 19:05 ` David Miller
-1 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2017-02-07 19:05 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa, bridge, colin.king
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Tue, 7 Feb 2017 12:46:46 +0100
> These checks should go after the attributes have been parsed otherwise
> we're using tb uninitialized.
>
> Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
> Reported-by: Colin Ian King <colin.king@canonical.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net-next] bridge: tunnel: fix attribute checks in br_parse_vlan_tunnel_info
@ 2017-02-07 19:05 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2017-02-07 19:05 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa, bridge, colin.king
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Tue, 7 Feb 2017 12:46:46 +0100
> These checks should go after the attributes have been parsed otherwise
> we're using tb uninitialized.
>
> Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
> Reported-by: Colin Ian King <colin.king@canonical.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-02-07 19:05 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-07 11:25 [Bridge] bridge: per vlan dst_metadata netlink support Colin Ian King
2017-02-07 11:25 ` Colin Ian King
2017-02-07 11:29 ` [Bridge] " Nikolay Aleksandrov
2017-02-07 11:29 ` Nikolay Aleksandrov via Bridge
2017-02-07 11:46 ` [Bridge] [PATCH net-next] bridge: tunnel: fix attribute checks in br_parse_vlan_tunnel_info Nikolay Aleksandrov
2017-02-07 11:46 ` Nikolay Aleksandrov
2017-02-07 13:24 ` [Bridge] " Roopa Prabhu
2017-02-07 13:24 ` Roopa Prabhu
2017-02-07 19:05 ` [Bridge] " David Miller
2017-02-07 19:05 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.