commit 0622cab0341c ("bonding: fix 802.3ad aggregator reselection"),
resolve case, when there is several aggregation groups in the same bond.
bond_3ad_unbind_slave will invalidate (clear) aggregator when
__agg_active_ports return zero. So, ad_clear_agg can be executed even, when
num_of_ports!=0. Than bond_3ad_unbind_slave can be executed again for,
previously cleared aggregator. NOTE: at this time bond_3ad_unbind_slave
will not update slave ports list, because lag_ports==NULL. So, here we
got slave ports, pointing to freed aggregator memory.
Fix with checking actual number of ports in group (as was before
commit 0622cab0341c ("bonding: fix 802.3ad aggregator reselection") ),
before ad_clear_agg().
The KASAN logs are as follows:
[ 767.617392] ==================================================================
[ 767.630776] BUG: KASAN: use-after-free in bond_3ad_state_machine_handler+0x13dc/0x1470
[ 767.638764] Read of size 2 at addr ffff00011ba9d430 by task kworker/u8:7/767
[ 767.647361] CPU: 3 PID: 767 Comm: kworker/u8:7 Tainted: G O 5.15.11 #15
[ 767.655329] Hardware name: DNI AmazonGo1 A7040 board (DT)
[ 767.660760] Workqueue: lacp_1 bond_3ad_state_machine_handler
[ 767.666468] Call trace:
[ 767.668930] dump_backtrace+0x0/0x2d0
[ 767.672625] show_stack+0x24/0x30
[ 767.675965] dump_stack_lvl+0x68/0x84
[ 767.679659] print_address_description.constprop.0+0x74/0x2b8
[ 767.685451] kasan_report+0x1f0/0x260
[ 767.689148] __asan_load2+0x94/0xd0
[ 767.692667] bond_3ad_state_machine_handler+0x13dc/0x1470
Fixes: 0622cab0341c ("bonding: fix 802.3ad aggregator reselection")
Co-developed-by: Maksym Glubokiy <[email protected]>
Signed-off-by: Maksym Glubokiy <[email protected]>
Signed-off-by: Yevhen Orlov <[email protected]>
---
drivers/net/bonding/bond_3ad.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index a86b1f71762e..d7fb33c078e8 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2228,7 +2228,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
temp_aggregator->num_of_ports--;
if (__agg_active_ports(temp_aggregator) == 0) {
select_new_active_agg = temp_aggregator->is_active;
- ad_clear_agg(temp_aggregator);
+ if (temp_aggregator->num_of_ports == 0)
+ ad_clear_agg(temp_aggregator);
if (select_new_active_agg) {
slave_info(bond->dev, slave->dev, "Removing an active aggregator\n");
/* select new active aggregator */
--
2.17.1
Yevhen Orlov <[email protected]> wrote:
>commit 0622cab0341c ("bonding: fix 802.3ad aggregator reselection"),
>resolve case, when there is several aggregation groups in the same bond.
>bond_3ad_unbind_slave will invalidate (clear) aggregator when
>__agg_active_ports return zero. So, ad_clear_agg can be executed even, when
>num_of_ports!=0. Than bond_3ad_unbind_slave can be executed again for,
>previously cleared aggregator. NOTE: at this time bond_3ad_unbind_slave
>will not update slave ports list, because lag_ports==NULL. So, here we
>got slave ports, pointing to freed aggregator memory.
>
>Fix with checking actual number of ports in group (as was before
>commit 0622cab0341c ("bonding: fix 802.3ad aggregator reselection") ),
>before ad_clear_agg().
To be clear, what it looks like is going on is that, after
0622cab0341c, we're getting to this point with an aggregator that
contains the port being removed and a non-zero number of inactive ports.
The extant logic is for the "old way" (no inactive ports in an agg), and
presumes that if __agg_active_ports() == 0 then the agg is empty, which
isn't a safe assumption in the current code.
Acked-by: Jay Vosburgh <[email protected]>
-J
>The KASAN logs are as follows:
>
>[ 767.617392] ==================================================================
>[ 767.630776] BUG: KASAN: use-after-free in bond_3ad_state_machine_handler+0x13dc/0x1470
>[ 767.638764] Read of size 2 at addr ffff00011ba9d430 by task kworker/u8:7/767
>[ 767.647361] CPU: 3 PID: 767 Comm: kworker/u8:7 Tainted: G O 5.15.11 #15
>[ 767.655329] Hardware name: DNI AmazonGo1 A7040 board (DT)
>[ 767.660760] Workqueue: lacp_1 bond_3ad_state_machine_handler
>[ 767.666468] Call trace:
>[ 767.668930] dump_backtrace+0x0/0x2d0
>[ 767.672625] show_stack+0x24/0x30
>[ 767.675965] dump_stack_lvl+0x68/0x84
>[ 767.679659] print_address_description.constprop.0+0x74/0x2b8
>[ 767.685451] kasan_report+0x1f0/0x260
>[ 767.689148] __asan_load2+0x94/0xd0
>[ 767.692667] bond_3ad_state_machine_handler+0x13dc/0x1470
>
>Fixes: 0622cab0341c ("bonding: fix 802.3ad aggregator reselection")
>Co-developed-by: Maksym Glubokiy <[email protected]>
>Signed-off-by: Maksym Glubokiy <[email protected]>
>Signed-off-by: Yevhen Orlov <[email protected]>
>---
> drivers/net/bonding/bond_3ad.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index a86b1f71762e..d7fb33c078e8 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -2228,7 +2228,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
> temp_aggregator->num_of_ports--;
> if (__agg_active_ports(temp_aggregator) == 0) {
> select_new_active_agg = temp_aggregator->is_active;
>- ad_clear_agg(temp_aggregator);
>+ if (temp_aggregator->num_of_ports == 0)
>+ ad_clear_agg(temp_aggregator);
> if (select_new_active_agg) {
> slave_info(bond->dev, slave->dev, "Removing an active aggregator\n");
> /* select new active aggregator */
>--
>2.17.1
>
Hello:
This patch was applied to netdev/net.git (master)
by Jakub Kicinski <[email protected]>:
On Wed, 29 Jun 2022 04:29:14 +0300 you wrote:
> commit 0622cab0341c ("bonding: fix 802.3ad aggregator reselection"),
> resolve case, when there is several aggregation groups in the same bond.
> bond_3ad_unbind_slave will invalidate (clear) aggregator when
> __agg_active_ports return zero. So, ad_clear_agg can be executed even, when
> num_of_ports!=0. Than bond_3ad_unbind_slave can be executed again for,
> previously cleared aggregator. NOTE: at this time bond_3ad_unbind_slave
> will not update slave ports list, because lag_ports==NULL. So, here we
> got slave ports, pointing to freed aggregator memory.
>
> [...]
Here is the summary with links:
- [net] net: bonding: fix use-after-free after 802.3ad slave unbind
https://git.kernel.org/netdev/net/c/050133e1aa2c
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html