All of lore.kernel.org
 help / color / mirror / Atom feed
From: Garry McNulty <garrmcnu@gmail.com>
To: netdev@vger.kernel.org
Cc: stephen@networkplumber.org, davem@davemloft.net,
	jiri@resnulli.us, nikolay@cumulusnetworks.com,
	bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
	Garry McNulty <garrmcnu@gmail.com>
Subject: [PATCH] net: bridge: fix potential null pointer dereference on return from br_port_get_rtnl()
Date: Thu, 21 Jun 2018 21:14:27 +0100	[thread overview]
Message-ID: <20180621201427.4961-1-garrmcnu@gmail.com> (raw)

br_port_get_rtnl() can return NULL if the network device is not a bridge
port (IFF_BRIDGE_PORT flag not set). br_port_slave_changelink() and
br_port_fill_slave_info() callbacks dereference this pointer without
checking. Currently this is not a problem because slave devices always
set this flag. Add null check in case these conditions ever change.

Detected by CoverityScan, CID 1339613 ("Dereference null return value")

Signed-off-by: Garry McNulty <garrmcnu@gmail.com>
---
 net/bridge/br_netlink.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 9f5eb05b0373..b3ad135b7157 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -947,13 +947,14 @@ static int br_port_slave_changelink(struct net_device *brdev,
 				    struct netlink_ext_ack *extack)
 {
 	struct net_bridge *br = netdev_priv(brdev);
+	struct net_bridge_port *p = br_port_get_rtnl(dev);
 	int ret;
 
-	if (!data)
+	if (!data || !p)
 		return 0;
 
 	spin_lock_bh(&br->lock);
-	ret = br_setport(br_port_get_rtnl(dev), data);
+	ret = br_setport(p, data);
 	spin_unlock_bh(&br->lock);
 
 	return ret;
@@ -963,7 +964,9 @@ static int br_port_fill_slave_info(struct sk_buff *skb,
 				   const struct net_device *brdev,
 				   const struct net_device *dev)
 {
-	return br_port_fill_attrs(skb, br_port_get_rtnl(dev));
+	struct net_bridge_port *p = br_port_get_rtnl(dev);
+
+	return p ? br_port_fill_attrs(skb, p) : -EINVAL;
 }
 
 static size_t br_port_get_slave_size(const struct net_device *brdev,
-- 
2.14.4


WARNING: multiple messages have this Message-ID (diff)
From: Garry McNulty <garrmcnu@gmail.com>
To: netdev@vger.kernel.org
Cc: jiri@resnulli.us, nikolay@cumulusnetworks.com,
	bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
	Garry McNulty <garrmcnu@gmail.com>,
	davem@davemloft.net
Subject: [Bridge] [PATCH] net: bridge: fix potential null pointer dereference on return from br_port_get_rtnl()
Date: Thu, 21 Jun 2018 21:14:27 +0100	[thread overview]
Message-ID: <20180621201427.4961-1-garrmcnu@gmail.com> (raw)

br_port_get_rtnl() can return NULL if the network device is not a bridge
port (IFF_BRIDGE_PORT flag not set). br_port_slave_changelink() and
br_port_fill_slave_info() callbacks dereference this pointer without
checking. Currently this is not a problem because slave devices always
set this flag. Add null check in case these conditions ever change.

Detected by CoverityScan, CID 1339613 ("Dereference null return value")

Signed-off-by: Garry McNulty <garrmcnu@gmail.com>
---
 net/bridge/br_netlink.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 9f5eb05b0373..b3ad135b7157 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -947,13 +947,14 @@ static int br_port_slave_changelink(struct net_device *brdev,
 				    struct netlink_ext_ack *extack)
 {
 	struct net_bridge *br = netdev_priv(brdev);
+	struct net_bridge_port *p = br_port_get_rtnl(dev);
 	int ret;
 
-	if (!data)
+	if (!data || !p)
 		return 0;
 
 	spin_lock_bh(&br->lock);
-	ret = br_setport(br_port_get_rtnl(dev), data);
+	ret = br_setport(p, data);
 	spin_unlock_bh(&br->lock);
 
 	return ret;
@@ -963,7 +964,9 @@ static int br_port_fill_slave_info(struct sk_buff *skb,
 				   const struct net_device *brdev,
 				   const struct net_device *dev)
 {
-	return br_port_fill_attrs(skb, br_port_get_rtnl(dev));
+	struct net_bridge_port *p = br_port_get_rtnl(dev);
+
+	return p ? br_port_fill_attrs(skb, p) : -EINVAL;
 }
 
 static size_t br_port_get_slave_size(const struct net_device *brdev,
-- 
2.14.4


             reply	other threads:[~2018-06-21 20:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-21 20:14 Garry McNulty [this message]
2018-06-21 20:14 ` [Bridge] [PATCH] net: bridge: fix potential null pointer dereference on return from br_port_get_rtnl() Garry McNulty
2018-06-21 22:20 ` David Miller
2018-06-21 22:20   ` [Bridge] " David Miller
2018-06-21 23:21   ` Stephen Hemminger
2018-06-21 23:21     ` [Bridge] " Stephen Hemminger
2018-06-21 23:21     ` Stephen Hemminger
2018-06-21 23:35   ` Nikolay Aleksandrov
2018-06-21 23:35     ` [Bridge] " Nikolay Aleksandrov
2018-06-21 23:35     ` Nikolay Aleksandrov
2018-06-22 19:05     ` Garry McNulty
2018-06-22 19:05       ` [Bridge] " Garry McNulty

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180621201427.4961-1-garrmcnu@gmail.com \
    --to=garrmcnu@gmail.com \
    --cc=bridge@lists.linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=jiri@resnulli.us \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@cumulusnetworks.com \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.