* [Bridge] [PATCH] net: bridge: fix potential NULL pointer dereference
@ 2017-06-05 21:30 ` Gustavo A. R. Silva
0 siblings, 0 replies; 12+ messages in thread
From: Gustavo A. R. Silva @ 2017-06-05 21:30 UTC (permalink / raw)
To: Stephen Hemminger, David S. Miller
Cc: netdev, bridge, Gustavo A. R. Silva, linux-kernel
Add NULL check before dereferencing pointer _p_ inside br_afspec().
Addresses-Coverity-ID: 1401872
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
net/bridge/br_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 1e63ec4..ad85a9c 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -776,7 +776,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
goto out;
}
- if (afspec) {
+ if (p && afspec) {
err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
afspec, RTM_SETLINK);
}
--
2.5.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH] net: bridge: fix potential NULL pointer dereference
@ 2017-06-05 21:30 ` Gustavo A. R. Silva
0 siblings, 0 replies; 12+ messages in thread
From: Gustavo A. R. Silva @ 2017-06-05 21:30 UTC (permalink / raw)
To: Stephen Hemminger, David S. Miller
Cc: bridge, netdev, linux-kernel, Gustavo A. R. Silva
Add NULL check before dereferencing pointer _p_ inside br_afspec().
Addresses-Coverity-ID: 1401872
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
net/bridge/br_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 1e63ec4..ad85a9c 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -776,7 +776,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
goto out;
}
- if (afspec) {
+ if (p && afspec) {
err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
afspec, RTM_SETLINK);
}
--
2.5.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Bridge] [PATCH] net: bridge: fix potential NULL pointer dereference
2017-06-05 21:30 ` Gustavo A. R. Silva
@ 2017-06-05 22:05 ` Nikolay Aleksandrov
-1 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2017-06-05 22:05 UTC (permalink / raw)
To: Gustavo A. R. Silva, Stephen Hemminger, David S. Miller
Cc: netdev, roopa, bridge
On 06/06/17 00:30, Gustavo A. R. Silva wrote:
> Add NULL check before dereferencing pointer _p_ inside br_afspec().
>
> Addresses-Coverity-ID: 1401872
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> ---
> net/bridge/br_netlink.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 1e63ec4..ad85a9c 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -776,7 +776,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
> goto out;
> }
>
> - if (afspec) {
> + if (p && afspec) {
> err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
> afspec, RTM_SETLINK);
> }
>
While I see a possible issue with the new bridge tunnel code (+CC Roopa), this is
the wrong fix because there are legitimate use cases where p is null and br_afspec
is called.
We need to change the p->flags check in br_afspec()'s IFLA_BRIDGE_VLAN_TUNNEL_INFO case
to check for a NULL p first.
Thanks for the report!
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] net: bridge: fix potential NULL pointer dereference
@ 2017-06-05 22:05 ` Nikolay Aleksandrov
0 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2017-06-05 22:05 UTC (permalink / raw)
To: Gustavo A. R. Silva, Stephen Hemminger, David S. Miller
Cc: bridge, netdev, roopa
On 06/06/17 00:30, Gustavo A. R. Silva wrote:
> Add NULL check before dereferencing pointer _p_ inside br_afspec().
>
> Addresses-Coverity-ID: 1401872
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> ---
> net/bridge/br_netlink.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 1e63ec4..ad85a9c 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -776,7 +776,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags)
> goto out;
> }
>
> - if (afspec) {
> + if (p && afspec) {
> err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
> afspec, RTM_SETLINK);
> }
>
While I see a possible issue with the new bridge tunnel code (+CC Roopa), this is
the wrong fix because there are legitimate use cases where p is null and br_afspec
is called.
We need to change the p->flags check in br_afspec()'s IFLA_BRIDGE_VLAN_TUNNEL_INFO case
to check for a NULL p first.
Thanks for the report!
^ permalink raw reply [flat|nested] 12+ messages in thread* [Bridge] [PATCH net] net: bridge: fix a null pointer dereference in br_afspec
2017-06-05 22:05 ` Nikolay Aleksandrov
@ 2017-06-05 22:26 ` Nikolay Aleksandrov
-1 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2017-06-05 22:26 UTC (permalink / raw)
To: netdev; +Cc: Nikolay Aleksandrov, roopa, bridge, garsilva, davem
We might call br_afspec() with p == NULL which is a valid use case if
the action is on the bridge device itself, but the bridge tunnel code
dereferences the p pointer without checking, so check if p is null
first.
Reported-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/bridge/br_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 1e63ec466d7c..3bcda556971e 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -595,7 +595,7 @@ static int br_afspec(struct net_bridge *br,
err = 0;
switch (nla_type(attr)) {
case IFLA_BRIDGE_VLAN_TUNNEL_INFO:
- if (!(p->flags & BR_VLAN_TUNNEL))
+ if (!p || !(p->flags & BR_VLAN_TUNNEL))
return -EINVAL;
err = br_parse_vlan_tunnel_info(attr, &tinfo_curr);
if (err)
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net] net: bridge: fix a null pointer dereference in br_afspec
@ 2017-06-05 22:26 ` Nikolay Aleksandrov
0 siblings, 0 replies; 12+ messages in thread
From: Nikolay Aleksandrov @ 2017-06-05 22:26 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, garsilva, bridge, Nikolay Aleksandrov
We might call br_afspec() with p == NULL which is a valid use case if
the action is on the bridge device itself, but the bridge tunnel code
dereferences the p pointer without checking, so check if p is null
first.
Reported-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
net/bridge/br_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 1e63ec466d7c..3bcda556971e 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -595,7 +595,7 @@ static int br_afspec(struct net_bridge *br,
err = 0;
switch (nla_type(attr)) {
case IFLA_BRIDGE_VLAN_TUNNEL_INFO:
- if (!(p->flags & BR_VLAN_TUNNEL))
+ if (!p || !(p->flags & BR_VLAN_TUNNEL))
return -EINVAL;
err = br_parse_vlan_tunnel_info(attr, &tinfo_curr);
if (err)
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Bridge] [PATCH net] net: bridge: fix a null pointer dereference in br_afspec
2017-06-05 22:26 ` Nikolay Aleksandrov
@ 2017-06-06 5:28 ` Roopa Prabhu
-1 siblings, 0 replies; 12+ messages in thread
From: Roopa Prabhu @ 2017-06-06 5:28 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev@vger.kernel.org, bridge, garsilva, davem@davemloft.net
On Mon, Jun 5, 2017 at 3:26 PM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> We might call br_afspec() with p == NULL which is a valid use case if
> the action is on the bridge device itself, but the bridge tunnel code
> dereferences the p pointer without checking, so check if p is null
> first.
>
> Reported-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Thanks Nikolay.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net] net: bridge: fix a null pointer dereference in br_afspec
@ 2017-06-06 5:28 ` Roopa Prabhu
0 siblings, 0 replies; 12+ messages in thread
From: Roopa Prabhu @ 2017-06-06 5:28 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev@vger.kernel.org, bridge, garsilva, davem@davemloft.net
On Mon, Jun 5, 2017 at 3:26 PM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> We might call br_afspec() with p == NULL which is a valid use case if
> the action is on the bridge device itself, but the bridge tunnel code
> dereferences the p pointer without checking, so check if p is null
> first.
>
> Reported-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Thanks Nikolay.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Bridge] [PATCH net] net: bridge: fix a null pointer dereference in br_afspec
2017-06-05 22:26 ` Nikolay Aleksandrov
@ 2017-06-06 20:06 ` David Miller
-1 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2017-06-06 20:06 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa, bridge, garsilva
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Tue, 6 Jun 2017 01:26:24 +0300
> We might call br_afspec() with p == NULL which is a valid use case if
> the action is on the bridge device itself, but the bridge tunnel code
> dereferences the p pointer without checking, so check if p is null
> first.
>
> Reported-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Applied and queued up for -stable, thanks Nikolay.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net] net: bridge: fix a null pointer dereference in br_afspec
@ 2017-06-06 20:06 ` David Miller
0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2017-06-06 20:06 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa, stephen, garsilva, bridge
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Tue, 6 Jun 2017 01:26:24 +0300
> We might call br_afspec() with p == NULL which is a valid use case if
> the action is on the bridge device itself, but the bridge tunnel code
> dereferences the p pointer without checking, so check if p is null
> first.
>
> Reported-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Applied and queued up for -stable, thanks Nikolay.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Bridge] [PATCH] net: bridge: fix potential NULL pointer dereference
2017-06-05 22:05 ` Nikolay Aleksandrov
@ 2017-06-06 3:13 ` Gustavo A. R. Silva
-1 siblings, 0 replies; 12+ messages in thread
From: Gustavo A. R. Silva @ 2017-06-06 3:13 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, roopa, bridge, David S. Miller
Hi Nikolay,
Quoting Nikolay Aleksandrov <nikolay@cumulusnetworks.com>:
> On 06/06/17 00:30, Gustavo A. R. Silva wrote:
>> Add NULL check before dereferencing pointer _p_ inside br_afspec().
>>
>> Addresses-Coverity-ID: 1401872
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> ---
>> net/bridge/br_netlink.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 1e63ec4..ad85a9c 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -776,7 +776,7 @@ int br_setlink(struct net_device *dev, struct
>> nlmsghdr *nlh, u16 flags)
>> goto out;
>> }
>>
>> - if (afspec) {
>> + if (p && afspec) {
>> err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
>> afspec, RTM_SETLINK);
>> }
>>
>
> While I see a possible issue with the new bridge tunnel code (+CC
> Roopa), this is
> the wrong fix because there are legitimate use cases where p is null
> and br_afspec
> is called.
> We need to change the p->flags check in br_afspec()'s
> IFLA_BRIDGE_VLAN_TUNNEL_INFO case
> to check for a NULL p first.
>
You're right. I got it.
> Thanks for the report!
Sure thing, glad to help.
Thank you!
--
Gustavo A. R. Silva
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] net: bridge: fix potential NULL pointer dereference
@ 2017-06-06 3:13 ` Gustavo A. R. Silva
0 siblings, 0 replies; 12+ messages in thread
From: Gustavo A. R. Silva @ 2017-06-06 3:13 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Stephen Hemminger, David S. Miller, bridge, netdev, roopa
Hi Nikolay,
Quoting Nikolay Aleksandrov <nikolay@cumulusnetworks.com>:
> On 06/06/17 00:30, Gustavo A. R. Silva wrote:
>> Add NULL check before dereferencing pointer _p_ inside br_afspec().
>>
>> Addresses-Coverity-ID: 1401872
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> ---
>> net/bridge/br_netlink.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 1e63ec4..ad85a9c 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -776,7 +776,7 @@ int br_setlink(struct net_device *dev, struct
>> nlmsghdr *nlh, u16 flags)
>> goto out;
>> }
>>
>> - if (afspec) {
>> + if (p && afspec) {
>> err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
>> afspec, RTM_SETLINK);
>> }
>>
>
> While I see a possible issue with the new bridge tunnel code (+CC
> Roopa), this is
> the wrong fix because there are legitimate use cases where p is null
> and br_afspec
> is called.
> We need to change the p->flags check in br_afspec()'s
> IFLA_BRIDGE_VLAN_TUNNEL_INFO case
> to check for a NULL p first.
>
You're right. I got it.
> Thanks for the report!
Sure thing, glad to help.
Thank you!
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-06-06 20:06 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-05 21:30 [Bridge] [PATCH] net: bridge: fix potential NULL pointer dereference Gustavo A. R. Silva
2017-06-05 21:30 ` Gustavo A. R. Silva
2017-06-05 22:05 ` [Bridge] " Nikolay Aleksandrov
2017-06-05 22:05 ` Nikolay Aleksandrov
2017-06-05 22:26 ` [Bridge] [PATCH net] net: bridge: fix a null pointer dereference in br_afspec Nikolay Aleksandrov
2017-06-05 22:26 ` Nikolay Aleksandrov
2017-06-06 5:28 ` [Bridge] " Roopa Prabhu
2017-06-06 5:28 ` Roopa Prabhu
2017-06-06 20:06 ` [Bridge] " David Miller
2017-06-06 20:06 ` David Miller
2017-06-06 3:13 ` [Bridge] [PATCH] net: bridge: fix potential NULL pointer dereference Gustavo A. R. Silva
2017-06-06 3:13 ` Gustavo A. R. Silva
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.