Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Yevhen Orlov <yevhen.orlov@plvision.eu>
To:     netdev@vger.kernel.org
Cc:     Volodymyr Mytnyk <volodymyr.mytnyk@plvision.eu>,
        Taras Chornyi <taras.chornyi@plvision.eu>,
        Mickey Rachamim <mickeyr@marvell.com>,
        Serhiy Pshyk <serhiy.pshyk@plvision.eu>,
        Yevhen Orlov <yevhen.orlov@plvision.eu>,
        Maksym Glubokiy <maksym.glubokiy@plvision.eu>,
        Jay Vosburgh <j.vosburgh@gmail.com>,
        Veaceslav Falico <vfalico@gmail.com>,
        Andy Gospodarek <andy@greyhouse.net>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, linux-kernel@vger.kernel.org
Subject: [PATCH net] net: bonding: fix use-after-free after 802.3ad slave unbind
Date:   Wed, 29 Jun 2022 04:29:14 +0300
Message-Id: <20220629012914.361-1-yevhen.orlov@plvision.eu>
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?8tYZ0Uqk1Pc5FxKuVhxwzbUc9B+NWrvcBvN3qf5qzxoTzxmqnq1MKxvZHOyR?=
 =?us-ascii?Q?iRUAdsXqq0hnSnMTpmnmL4TEzGRFAlWEg31KqCPQfF6giSpN5+k8iliHA6Z7?=
 =?us-ascii?Q?DAeJUuykWCNVg7WejGyN4Vsh49Exg0xFuujlaFWUrXHTXe6kTchR/N4DIlDj?=
 =?us-ascii?Q?QIMpM9ISz7PvmM7/4ecPVNaE9SpMLpQe9b2rV9BZF8zr1tNw1s+D5xzWlm8t?=
 =?us-ascii?Q?IPDlzQQmSEttrnS1IazgB+igJbOxxoESxVsh1iqSuYgEei3ZW7V5oLnVKYCF?=
 =?us-ascii?Q?ZllAYEnqtI/CMpmKLblMkzUkMqq81VhIUlgItUjYaH83x5P9BoZmuYkkOWXE?=
 =?us-ascii?Q?mf0wBfrZGZ33/qqUa47mk3B9pl7Pm4R8u3RZ1Hd6p+kPKrS9c2dhD+4bCnBI?=
 =?us-ascii?Q?2gLj2MCo9KORd3uKJcwDQP8ov1f1Ym84JdCuQ9fcgQfZCTkICtDovaohi0dE?=
 =?us-ascii?Q?v53mDFuO4H06yVafyp5qdZXVkrotSwiV3Ij2c+Rea8wlV6QIdfZusQlo9DX1?=
 =?us-ascii?Q?+zFhpbh6f+goHbTmjW7OQAc1omRh3r/OrH+LDvF4+C/0wP7evJih6+U0bUa8?=
 =?us-ascii?Q?5923H7ZcksK5dBvWvutajufmfLpaxDjznBt8HGzu6s12s9RGHEgqhXkv+2Tg?=
 =?us-ascii?Q?5LrUl9E52UXWK8zN0U26WWq3NoyaqCuQ5Yuo+Uk5a6d0JGwC5YTv97qRsSJ+?=
 =?us-ascii?Q?bu8mnUVWnSWF+z+XKtz7KGuZ+nPbJeQHB/jjKpBwl2Gl5zZX2xAIBoyiF2DW?=
 =?us-ascii?Q?1Z0cqwgp90Kigms8gWh6vimiwTaEf6VUkaJyujT7nKFxYhnWIH9t5abBgt3p?=
 =?us-ascii?Q?7NMNk4YZ79dI1neCGtU319SYjHXxEyAhIIoAaeaHYxG5bcAdoYYGCDFWaJjP?=
 =?us-ascii?Q?4LRlAH0tRptiguyX6HNl/ViByOVH5MkTYFH0jmRJadqVji79/kjCfhAQiZwU?=
 =?us-ascii?Q?lcwH6WhJEiJNoOTJpJJkQMPxQrGg+2NGiYash0BGxhPICxXj3y1JzT31Ei8v?=
 =?us-ascii?Q?GAtqs5qBD+lJTMnMxZd06PSgbk60Y/VxF6hk4kbelGYa3KSRGt7zpIHj4KVX?=
 =?us-ascii?Q?a6f3Sx2C7CDXm1tbTSg1Z4a7W6MaF4lyENYwqjqEcV4hDYa8gaxpY8VPJuqK?=
 =?us-ascii?Q?H4miw9KwUj/56oZespgVB3Xe0Cci+7cvk1oZkIuu4nvFoWAnlXckQwPQfXUp?=
 =?us-ascii?Q?rB16CD2+Kxb3HBBvd2neJVSWskDquqbPpwwEo3IxBdeXkeARIgrIjEwshC0W?=
 =?us-ascii?Q?qzXDCFqcrWrf5N/HSqzOPWFm40wp/zZnonm8BQLgL3Ye1OBZhkU1pJGLGlzg?=
 =?us-ascii?Q?PcGi2h4hPzMQTklfzCDWxQfQvQckP44ZHVlkhPtyBnciMoHL08lxpCiZyArI?=
 =?us-ascii?Q?zy1EwTTsQujus1tdniwgiwY6NRvksH7fG9i/Vzo0K8+gbxuGLmVFcRjN2lmO?=
 =?us-ascii?Q?dsBdhnfXHUHhP56ZQ2inCeEemZ26P5PKvEv1PtmOVaJ7U5c1jVb9KFIHf415?=
 =?us-ascii?Q?5vLdt0ZLAe/Fdqk3dReMSr9f1A4UDrgBGz5IT0F4BMIJiHV4uc5TBubN38FE?=
 =?us-ascii?Q?bJLDMhf+cxPAhJdqMxU7MVmcTp3VYJWhxXBmospQu+AsT2RNIrP3xbBv2sPi?=
 =?us-ascii?Q?vw=3D=3D?=
X-OriginatorOrg: plvision.eu
X-MS-Exchange-CrossTenant-Network-Message-Id: 88555e20-055c-4a0e-112f-08da596edc0c
X-MS-Exchange-CrossTenant-AuthSource: AS4P190MB1782.EURP190.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jun 2022 01:29:50.0649
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 03707b74-30f3-46b6-a0e0-ff0a7438c9c4
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: +aMojuJZTsQ5bonpkRWaDQJSw9P4uz4XAo3jqOPdiSBbqenZ3O6em9xoymO/Grt9CFOEe+RgeFGPB8KUx8TuYqgkJZ0cXs3xkBtZT2vSkK0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6P190MB0408
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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 <maksym.glubokiy@plvision.eu>
Signed-off-by: Maksym Glubokiy <maksym.glubokiy@plvision.eu>
Signed-off-by: Yevhen Orlov <yevhen.orlov@plvision.eu>
---
 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