* [PATCH] bonding: fix race condition in bonding_store_slaves_active
@ 2012-11-24 12:19 Nikolay Aleksandrov
2012-11-28 16:21 ` David Miller
2012-11-29 11:37 ` [PATCH v2] " Nikolay Aleksandrov
0 siblings, 2 replies; 5+ messages in thread
From: Nikolay Aleksandrov @ 2012-11-24 12:19 UTC (permalink / raw)
To: netdev; +Cc: andy, fubar, davem
Race between bonding_store_slaves_active() and slave manipulation functions
The bond_for_each_slave use in bonding_store_slaves_active() is not protected
by any synchronization mechanism. NULL pointer dereference is easy to reach.
Fixed by acquiring the bond->lock for the slave walk.
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_sysfs.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index ef8d2a0..ba4f95b 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1582,6 +1582,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
goto out;
}
+ read_lock(&bond->lock);
bond_for_each_slave(bond, slave, i) {
if (!bond_is_active_slave(slave)) {
if (new_value)
@@ -1590,6 +1591,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
slave->inactive = 1;
}
}
+ read_unlock(&bond->lock);
out:
return ret;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] bonding: fix race condition in bonding_store_slaves_active
2012-11-24 12:19 [PATCH] bonding: fix race condition in bonding_store_slaves_active Nikolay Aleksandrov
@ 2012-11-28 16:21 ` David Miller
2012-11-29 11:37 ` [PATCH v2] " Nikolay Aleksandrov
1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2012-11-28 16:21 UTC (permalink / raw)
To: nikolay; +Cc: netdev, andy, fubar
Some bonding developer reviews would be appreciated.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] bonding: fix race condition in bonding_store_slaves_active
2012-11-24 12:19 [PATCH] bonding: fix race condition in bonding_store_slaves_active Nikolay Aleksandrov
2012-11-28 16:21 ` David Miller
@ 2012-11-29 11:37 ` Nikolay Aleksandrov
2012-11-29 17:37 ` Jay Vosburgh
1 sibling, 1 reply; 5+ messages in thread
From: Nikolay Aleksandrov @ 2012-11-29 11:37 UTC (permalink / raw)
To: netdev; +Cc: fubar, andy, davem
Race between bonding_store_slaves_active() and slave manipulation
functions. The bond_for_each_slave use in bonding_store_slaves_active()
is not protected by any synchronization mechanism.
NULL pointer dereference is easy to reach.
Fixed by acquiring the bond->lock for the slave walk.
v2: Make description text < 75 columns
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_sysfs.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index ef8d2a0..ba4f95b 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1582,6 +1582,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
goto out;
}
+ read_lock(&bond->lock);
bond_for_each_slave(bond, slave, i) {
if (!bond_is_active_slave(slave)) {
if (new_value)
@@ -1590,6 +1591,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
slave->inactive = 1;
}
}
+ read_unlock(&bond->lock);
out:
return ret;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] bonding: fix race condition in bonding_store_slaves_active
2012-11-29 11:37 ` [PATCH v2] " Nikolay Aleksandrov
@ 2012-11-29 17:37 ` Jay Vosburgh
2012-11-29 18:14 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Jay Vosburgh @ 2012-11-29 17:37 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, andy, davem
Nikolay Aleksandrov <nikolay@redhat.com> wrote:
> Race between bonding_store_slaves_active() and slave manipulation
> functions. The bond_for_each_slave use in bonding_store_slaves_active()
> is not protected by any synchronization mechanism.
> NULL pointer dereference is easy to reach.
> Fixed by acquiring the bond->lock for the slave walk.
>
> v2: Make description text < 75 columns
>
>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
>---
> drivers/net/bonding/bond_sysfs.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index ef8d2a0..ba4f95b 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -1582,6 +1582,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
> goto out;
> }
>
>+ read_lock(&bond->lock);
> bond_for_each_slave(bond, slave, i) {
> if (!bond_is_active_slave(slave)) {
> if (new_value)
>@@ -1590,6 +1591,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
> slave->inactive = 1;
> }
> }
>+ read_unlock(&bond->lock);
> out:
> return ret;
> }
>--
>1.7.11.7
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] bonding: fix race condition in bonding_store_slaves_active
2012-11-29 17:37 ` Jay Vosburgh
@ 2012-11-29 18:14 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2012-11-29 18:14 UTC (permalink / raw)
To: fubar; +Cc: nikolay, netdev, andy
From: Jay Vosburgh <fubar@us.ibm.com>
Date: Thu, 29 Nov 2012 09:37:56 -0800
> Nikolay Aleksandrov <nikolay@redhat.com> wrote:
>
>> Race between bonding_store_slaves_active() and slave manipulation
>> functions. The bond_for_each_slave use in bonding_store_slaves_active()
>> is not protected by any synchronization mechanism.
>> NULL pointer dereference is easy to reach.
>> Fixed by acquiring the bond->lock for the slave walk.
>>
>> v2: Make description text < 75 columns
>>
>>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>
> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
Applied.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-11-29 18:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-24 12:19 [PATCH] bonding: fix race condition in bonding_store_slaves_active Nikolay Aleksandrov
2012-11-28 16:21 ` David Miller
2012-11-29 11:37 ` [PATCH v2] " Nikolay Aleksandrov
2012-11-29 17:37 ` Jay Vosburgh
2012-11-29 18:14 ` 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).