* [PATCHv2 0/2] bonding: NULL pointer dereference and flag strip fixes
@ 2013-04-11 19:18 Nikolay Aleksandrov
2013-04-11 19:18 ` [PATCHv2 1/2] bonding: fix netdev event NULL pointer dereference Nikolay Aleksandrov
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2013-04-11 19:18 UTC (permalink / raw)
To: netdev; +Cc: andy, fubar, davem
These 2 patches fix a NULL pointer dereference in the netdev event
handling of the bond and flag that wasn't stripped on enslave failure.
v2: Fix comment style
Nikolay Aleksandrov (2):
bonding: fix netdev event NULL pointer dereference
bonding: IFF_BONDING is not stripped on enslave failure
drivers/net/bonding/bond_main.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
--
1.8.1.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCHv2 1/2] bonding: fix netdev event NULL pointer dereference
2013-04-11 19:18 [PATCHv2 0/2] bonding: NULL pointer dereference and flag strip fixes Nikolay Aleksandrov
@ 2013-04-11 19:18 ` Nikolay Aleksandrov
2013-04-11 19:18 ` [PATCHv2 2/2] bonding: IFF_BONDING is not stripped on enslave failure Nikolay Aleksandrov
2013-04-11 20:02 ` [PATCHv2 0/2] bonding: NULL pointer dereference and flag strip fixes David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2013-04-11 19:18 UTC (permalink / raw)
To: netdev; +Cc: andy, fubar, davem
In commit 471cb5a33dcbd7c529684a2ac7ba4451414ee4a7 ("bonding: remove
usage of dev->master") a bug was introduced which causes a NULL pointer
dereference. If a bond device is in mode 6 (ALB) and a slave is added
it will dereference a NULL pointer in bond_slave_netdev_event().
This is because in bond_enslave we have bond_alb_init_slave() which
changes the MAC address of the slave and causes a NETDEV_CHANGEADDR.
Then we have in bond_slave_netdev_event():
struct slave *slave = bond_slave_get_rtnl(slave_dev);
struct bonding *bond = slave->bond;
bond_slave_get_rtnl() dereferences slave_dev->rx_handler_data which at
that time is NULL since netdev_rx_handler_register() is called later.
This is fixed by checking if slave is NULL before dereferencing it.
v2: Comment style changed.
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_main.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 171b10f..15c675c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3168,11 +3168,20 @@ static int bond_slave_netdev_event(unsigned long event,
struct net_device *slave_dev)
{
struct slave *slave = bond_slave_get_rtnl(slave_dev);
- struct bonding *bond = slave->bond;
- struct net_device *bond_dev = slave->bond->dev;
+ struct bonding *bond;
+ struct net_device *bond_dev;
u32 old_speed;
u8 old_duplex;
+ /* A netdev event can be generated while enslaving a device
+ * before netdev_rx_handler_register is called in which case
+ * slave will be NULL
+ */
+ if (!slave)
+ return NOTIFY_DONE;
+ bond_dev = slave->bond->dev;
+ bond = slave->bond;
+
switch (event) {
case NETDEV_UNREGISTER:
if (bond->setup_by_slave)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCHv2 2/2] bonding: IFF_BONDING is not stripped on enslave failure
2013-04-11 19:18 [PATCHv2 0/2] bonding: NULL pointer dereference and flag strip fixes Nikolay Aleksandrov
2013-04-11 19:18 ` [PATCHv2 1/2] bonding: fix netdev event NULL pointer dereference Nikolay Aleksandrov
@ 2013-04-11 19:18 ` Nikolay Aleksandrov
2013-04-11 20:02 ` [PATCHv2 0/2] bonding: NULL pointer dereference and flag strip fixes David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2013-04-11 19:18 UTC (permalink / raw)
To: netdev; +Cc: andy, fubar, davem
While enslaving a new device and after IFF_BONDING flag is set, in case
of failure it is not stripped from the device's priv_flags while
cleaning up, which could lead to other problems.
Cleaning at err_close because the flag is set after dev_open().
v2: no change
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 15c675c..232b129 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1906,6 +1906,7 @@ err_detach:
write_unlock_bh(&bond->lock);
err_close:
+ slave_dev->priv_flags &= ~IFF_BONDING;
dev_close(slave_dev);
err_unset_master:
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCHv2 0/2] bonding: NULL pointer dereference and flag strip fixes
2013-04-11 19:18 [PATCHv2 0/2] bonding: NULL pointer dereference and flag strip fixes Nikolay Aleksandrov
2013-04-11 19:18 ` [PATCHv2 1/2] bonding: fix netdev event NULL pointer dereference Nikolay Aleksandrov
2013-04-11 19:18 ` [PATCHv2 2/2] bonding: IFF_BONDING is not stripped on enslave failure Nikolay Aleksandrov
@ 2013-04-11 20:02 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-04-11 20:02 UTC (permalink / raw)
To: nikolay; +Cc: netdev, andy, fubar
From: Nikolay Aleksandrov <nikolay@redhat.com>
Date: Thu, 11 Apr 2013 21:18:54 +0200
> These 2 patches fix a NULL pointer dereference in the netdev event
> handling of the bond and flag that wasn't stripped on enslave failure.
>
> v2: Fix comment style
>
> Nikolay Aleksandrov (2):
> bonding: fix netdev event NULL pointer dereference
> bonding: IFF_BONDING is not stripped on enslave failure
Both applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] bonding: fix netdev event NULL pointer dereference
@ 2013-04-09 20:17 Nikolay Aleksandrov
2013-04-11 13:45 ` [PATCHv2 " Nikolay Aleksandrov
0 siblings, 1 reply; 6+ messages in thread
From: Nikolay Aleksandrov @ 2013-04-09 20:17 UTC (permalink / raw)
To: netdev; +Cc: andy, fubar, davem
In commit 471cb5a33dcbd7c529684a2ac7ba4451414ee4a7 ("bonding: remove
usage of dev->master") a bug was introduced which causes a NULL pointer
dereference. If a bond device is in mode 6 (ALB) and a slave is added
it will dereference a NULL pointer in bond_slave_netdev_event().
This is because in bond_enslave we have bond_alb_init_slave() which
changes the MAC address of the slave and causes a NETDEV_CHANGEADDR.
Then we have in bond_slave_netdev_event():
struct slave *slave = bond_slave_get_rtnl(slave_dev);
struct bonding *bond = slave->bond;
bond_slave_get_rtnl() dereferences slave_dev->rx_handler_data which at
that time is NULL since netdev_rx_handler_register() is called later.
This is fixed by checking if slave is NULL before dereferencing it.
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_main.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 171b10f..9995ddc 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3168,11 +3168,21 @@ static int bond_slave_netdev_event(unsigned long event,
struct net_device *slave_dev)
{
struct slave *slave = bond_slave_get_rtnl(slave_dev);
- struct bonding *bond = slave->bond;
- struct net_device *bond_dev = slave->bond->dev;
+ struct bonding *bond;
+ struct net_device *bond_dev;
u32 old_speed;
u8 old_duplex;
+ /*
+ * A netdev event can be generated while enslaving a device
+ * before netdev_rx_handler_register is called in which case
+ * slave will be NULL
+ */
+ if (!slave)
+ return NOTIFY_DONE;
+ bond_dev = slave->bond->dev;
+ bond = slave->bond;
+
switch (event) {
case NETDEV_UNREGISTER:
if (bond->setup_by_slave)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCHv2 1/2] bonding: fix netdev event NULL pointer dereference
2013-04-09 20:17 [PATCH 1/2] bonding: fix netdev event NULL pointer dereference Nikolay Aleksandrov
@ 2013-04-11 13:45 ` Nikolay Aleksandrov
2013-04-11 17:28 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Nikolay Aleksandrov @ 2013-04-11 13:45 UTC (permalink / raw)
To: netdev; +Cc: andy, fubar, davem
In commit 471cb5a33dcbd7c529684a2ac7ba4451414ee4a7 ("bonding: remove
usage of dev->master") a bug was introduced which causes a NULL pointer
dereference. If a bond device is in mode 6 (ALB) and a slave is added
it will dereference a NULL pointer in bond_slave_netdev_event().
This is because in bond_enslave we have bond_alb_init_slave() which
changes the MAC address of the slave and causes a NETDEV_CHANGEADDR.
Then we have in bond_slave_netdev_event():
struct slave *slave = bond_slave_get_rtnl(slave_dev);
struct bonding *bond = slave->bond;
bond_slave_get_rtnl() dereferences slave_dev->rx_handler_data which at
that time is NULL since netdev_rx_handler_register() is called later.
This is fixed by checking if slave is NULL before dereferencing it.
v2: Fix the comment styling
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_main.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 171b10f..15c675c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3168,11 +3168,20 @@ static int bond_slave_netdev_event(unsigned long event,
struct net_device *slave_dev)
{
struct slave *slave = bond_slave_get_rtnl(slave_dev);
- struct bonding *bond = slave->bond;
- struct net_device *bond_dev = slave->bond->dev;
+ struct bonding *bond;
+ struct net_device *bond_dev;
u32 old_speed;
u8 old_duplex;
+ /* A netdev event can be generated while enslaving a device
+ * before netdev_rx_handler_register is called in which case
+ * slave will be NULL
+ */
+ if (!slave)
+ return NOTIFY_DONE;
+ bond_dev = slave->bond->dev;
+ bond = slave->bond;
+
switch (event) {
case NETDEV_UNREGISTER:
if (bond->setup_by_slave)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCHv2 1/2] bonding: fix netdev event NULL pointer dereference
2013-04-11 13:45 ` [PATCHv2 " Nikolay Aleksandrov
@ 2013-04-11 17:28 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-04-11 17:28 UTC (permalink / raw)
To: nikolay; +Cc: netdev, andy, fubar
When you repost a patch that's part of a series, you must resubmit all
patches in that series, not just the one that changed.
You should also provide a header posting "[PATCH 0/N] ..." that explains
the high level purpose and intent of the patch set.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-11 20:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-11 19:18 [PATCHv2 0/2] bonding: NULL pointer dereference and flag strip fixes Nikolay Aleksandrov
2013-04-11 19:18 ` [PATCHv2 1/2] bonding: fix netdev event NULL pointer dereference Nikolay Aleksandrov
2013-04-11 19:18 ` [PATCHv2 2/2] bonding: IFF_BONDING is not stripped on enslave failure Nikolay Aleksandrov
2013-04-11 20:02 ` [PATCHv2 0/2] bonding: NULL pointer dereference and flag strip fixes David Miller
-- strict thread matches above, loose matches on Subject: below --
2013-04-09 20:17 [PATCH 1/2] bonding: fix netdev event NULL pointer dereference Nikolay Aleksandrov
2013-04-11 13:45 ` [PATCHv2 " Nikolay Aleksandrov
2013-04-11 17:28 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).