| Message ID | 20231218070850.15870-1-xiaoke@sangfor.com.cn |
|---|---|
| State | Accepted |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | [net,v2] i40e: fix use-after-free in i40e_aqc_add_filters() | expand |
On 12/17/2023 11:08 PM, Ke Xiao wrote: > Commit 3116f59c12bd ("i40e: fix use-after-free in > i40e_sync_filters_subtask()") avoided use-after-free issues, > by increasing refcount during update the VSI filter list to > the HW. However, it missed the unicast situation. > > When deleting an unicast FDB entry, the i40e driver will release > the mac_filter, and i40e_service_task will concurrently request > firmware to add the mac_filter, which will lead to the following > use-after-free issue. > > Fix again for both netdev->uc and netdev->mc. > Thanks for fixing this! Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > BUG: KASAN: use-after-free in i40e_aqc_add_filters+0x55c/0x5b0 [i40e] > Read of size 2 at addr ffff888eb3452d60 by task kworker/8:7/6379 > > CPU: 8 PID: 6379 Comm: kworker/8:7 Kdump: loaded Tainted: G > Workqueue: i40e i40e_service_task [i40e] > Call Trace: > dump_stack+0x71/0xab > print_address_description+0x6b/0x290 > kasan_report+0x14a/0x2b0 > i40e_aqc_add_filters+0x55c/0x5b0 [i40e] > i40e_sync_vsi_filters+0x1676/0x39c0 [i40e] > i40e_service_task+0x1397/0x2bb0 [i40e] > process_one_work+0x56a/0x11f0 > worker_thread+0x8f/0xf40 > kthread+0x2a0/0x390 > ret_from_fork+0x1f/0x40 > > Allocated by task 21948: > kasan_kmalloc+0xa6/0xd0 > kmem_cache_alloc_trace+0xdb/0x1c0 > i40e_add_filter+0x11e/0x520 [i40e] > i40e_addr_sync+0x37/0x60 [i40e] > __hw_addr_sync_dev+0x1f5/0x2f0 > i40e_set_rx_mode+0x61/0x1e0 [i40e] > dev_uc_add_excl+0x137/0x190 > i40e_ndo_fdb_add+0x161/0x260 [i40e] > rtnl_fdb_add+0x567/0x950 > rtnetlink_rcv_msg+0x5db/0x880 > netlink_rcv_skb+0x254/0x380 > netlink_unicast+0x454/0x610 > netlink_sendmsg+0x747/0xb00 > sock_sendmsg+0xe2/0x120 > __sys_sendto+0x1ae/0x290 > __x64_sys_sendto+0xdd/0x1b0 > do_syscall_64+0xa0/0x370 > entry_SYSCALL_64_after_hwframe+0x65/0xca > > Freed by task 21948: > __kasan_slab_free+0x137/0x190 > kfree+0x8b/0x1b0 > __i40e_del_filter+0x116/0x1e0 [i40e] > i40e_del_mac_filter+0x16c/0x300 [i40e] > i40e_addr_unsync+0x134/0x1b0 [i40e] > __hw_addr_sync_dev+0xff/0x2f0 > i40e_set_rx_mode+0x61/0x1e0 [i40e] > dev_uc_del+0x77/0x90 > rtnl_fdb_del+0x6a5/0x860 > rtnetlink_rcv_msg+0x5db/0x880 > netlink_rcv_skb+0x254/0x380 > netlink_unicast+0x454/0x610 > netlink_sendmsg+0x747/0xb00 > sock_sendmsg+0xe2/0x120 > __sys_sendto+0x1ae/0x290 > __x64_sys_sendto+0xdd/0x1b0 > do_syscall_64+0xa0/0x370 > entry_SYSCALL_64_after_hwframe+0x65/0xca > > Fixes: 3116f59c12bd ("i40e: fix use-after-free in i40e_sync_filters_subtask()") > Fixes: 41c445ff0f48 ("i40e: main driver core") > Signed-off-by: Ke Xiao <xiaoke@sangfor.com.cn> > Signed-off-by: Ding Hui <dinghui@sangfor.com.cn> > Cc: Di Zhu <zhudi2@huawei.com> > Reviewed-by: Jan Sokolowski <jan.sokolowski@intel.com> > Reviewed-by: Simon Horman <horms@kernel.org> > --- > v2: > - Order local variable declarations in Reverse Christmas Tree (RCT) > > --- > drivers/net/ethernet/intel/i40e/i40e_main.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c > index 1ab8dbe2d880..d5633a440cca 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c > @@ -107,12 +107,18 @@ static struct workqueue_struct *i40e_wq; > static void netdev_hw_addr_refcnt(struct i40e_mac_filter *f, > struct net_device *netdev, int delta) > { > + struct netdev_hw_addr_list *ha_list; > struct netdev_hw_addr *ha; > > if (!f || !netdev) > return; > > - netdev_for_each_mc_addr(ha, netdev) { > + if (is_unicast_ether_addr(f->macaddr) || is_link_local_ether_addr(f->macaddr)) > + ha_list = &netdev->uc; > + else > + ha_list = &netdev->mc; > + > + netdev_hw_addr_list_for_each(ha, ha_list) { > if (ether_addr_equal(ha->addr, f->macaddr)) { > ha->refcount += delta; > if (ha->refcount <= 0)
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Ke Xiao > Sent: Monday, December 18, 2023 12:39 PM > To: Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com > Cc: Sokolowski, Jan <jan.sokolowski@intel.com>; netdev@vger.kernel.org; Ding, Hui <dinghui@sangfor.com.cn>; linux-kernel@vger.kernel.org; zhangrui182@huawei.com; intel-wired-lan@lists.osuosl.org; horms@kernel.org; zhudi2@huawei.com; Ke Xiao <xiaoke@sangfor.com.cn>; shannon.nelson@amd.com > Subject: [Intel-wired-lan] [net PATCH v2] i40e: fix use-after-free in i40e_aqc_add_filters() > > Commit 3116f59c12bd ("i40e: fix use-after-free in > i40e_sync_filters_subtask()") avoided use-after-free issues, > by increasing refcount during update the VSI filter list to > the HW. However, it missed the unicast situation. > > When deleting an unicast FDB entry, the i40e driver will release > the mac_filter, and i40e_service_task will concurrently request > firmware to add the mac_filter, which will lead to the following > use-after-free issue. > > Fix again for both netdev->uc and netdev->mc. > > BUG: KASAN: use-after-free in i40e_aqc_add_filters+0x55c/0x5b0 [i40e] > Read of size 2 at addr ffff888eb3452d60 by task kworker/8:7/6379 > > CPU: 8 PID: 6379 Comm: kworker/8:7 Kdump: loaded Tainted: G > Workqueue: i40e i40e_service_task [i40e] > Call Trace: > dump_stack+0x71/0xab > print_address_description+0x6b/0x290 > kasan_report+0x14a/0x2b0 > i40e_aqc_add_filters+0x55c/0x5b0 [i40e] > i40e_sync_vsi_filters+0x1676/0x39c0 [i40e] > i40e_service_task+0x1397/0x2bb0 [i40e] > process_one_work+0x56a/0x11f0 > worker_thread+0x8f/0xf40 > kthread+0x2a0/0x390 > ret_from_fork+0x1f/0x40 > > Allocated by task 21948: > kasan_kmalloc+0xa6/0xd0 > kmem_cache_alloc_trace+0xdb/0x1c0 > i40e_add_filter+0x11e/0x520 [i40e] > i40e_addr_sync+0x37/0x60 [i40e] > __hw_addr_sync_dev+0x1f5/0x2f0 > i40e_set_rx_mode+0x61/0x1e0 [i40e] > dev_uc_add_excl+0x137/0x190 > i40e_ndo_fdb_add+0x161/0x260 [i40e] > rtnl_fdb_add+0x567/0x950 > rtnetlink_rcv_msg+0x5db/0x880 > netlink_rcv_skb+0x254/0x380 > netlink_unicast+0x454/0x610 > netlink_sendmsg+0x747/0xb00 > sock_sendmsg+0xe2/0x120 > __sys_sendto+0x1ae/0x290 > __x64_sys_sendto+0xdd/0x1b0 > do_syscall_64+0xa0/0x370 > entry_SYSCALL_64_after_hwframe+0x65/0xca > > Freed by task 21948: > __kasan_slab_free+0x137/0x190 > kfree+0x8b/0x1b0 > __i40e_del_filter+0x116/0x1e0 [i40e] > i40e_del_mac_filter+0x16c/0x300 [i40e] > i40e_addr_unsync+0x134/0x1b0 [i40e] > __hw_addr_sync_dev+0xff/0x2f0 > i40e_set_rx_mode+0x61/0x1e0 [i40e] > dev_uc_del+0x77/0x90 > rtnl_fdb_del+0x6a5/0x860 > rtnetlink_rcv_msg+0x5db/0x880 > netlink_rcv_skb+0x254/0x380 > netlink_unicast+0x454/0x610 > netlink_sendmsg+0x747/0xb00 > sock_sendmsg+0xe2/0x120 > __sys_sendto+0x1ae/0x290 > __x64_sys_sendto+0xdd/0x1b0 > do_syscall_64+0xa0/0x370 > entry_SYSCALL_64_after_hwframe+0x65/0xca > > Fixes: 3116f59c12bd ("i40e: fix use-after-free in i40e_sync_filters_subtask()") > Fixes: 41c445ff0f48 ("i40e: main driver core") > Signed-off-by: Ke Xiao <xiaoke@sangfor.com.cn> > Signed-off-by: Ding Hui <dinghui@sangfor.com.cn> > Cc: Di Zhu <zhudi2@huawei.com> > Reviewed-by: Jan Sokolowski <jan.sokolowski@intel.com> > Reviewed-by: Simon Horman <horms@kernel.org> > --- > v2: > - Order local variable declarations in Reverse Christmas Tree (RCT) > > --- > drivers/net/ethernet/intel/i40e/i40e_main.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 1ab8dbe2d880..d5633a440cca 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -107,12 +107,18 @@ static struct workqueue_struct *i40e_wq; static void netdev_hw_addr_refcnt(struct i40e_mac_filter *f, struct net_device *netdev, int delta) { + struct netdev_hw_addr_list *ha_list; struct netdev_hw_addr *ha; if (!f || !netdev) return; - netdev_for_each_mc_addr(ha, netdev) { + if (is_unicast_ether_addr(f->macaddr) || is_link_local_ether_addr(f->macaddr)) + ha_list = &netdev->uc; + else + ha_list = &netdev->mc; + + netdev_hw_addr_list_for_each(ha, ha_list) { if (ether_addr_equal(ha->addr, f->macaddr)) { ha->refcount += delta; if (ha->refcount <= 0)
Commit 3116f59c12bd ("i40e: fix use-after-free in i40e_sync_filters_subtask()") avoided use-after-free issues, by increasing refcount during update the VSI filter list to the HW. However, it missed the unicast situation. When deleting an unicast FDB entry, the i40e driver will release the mac_filter, and i40e_service_task will concurrently request firmware to add the mac_filter, which will lead to the following use-after-free issue. Fix again for both netdev->uc and netdev->mc. BUG: KASAN: use-after-free in i40e_aqc_add_filters+0x55c/0x5b0 [i40e] Read of size 2 at addr ffff888eb3452d60 by task kworker/8:7/6379 CPU: 8 PID: 6379 Comm: kworker/8:7 Kdump: loaded Tainted: G Workqueue: i40e i40e_service_task [i40e] Call Trace: dump_stack+0x71/0xab print_address_description+0x6b/0x290 kasan_report+0x14a/0x2b0 i40e_aqc_add_filters+0x55c/0x5b0 [i40e] i40e_sync_vsi_filters+0x1676/0x39c0 [i40e] i40e_service_task+0x1397/0x2bb0 [i40e] process_one_work+0x56a/0x11f0 worker_thread+0x8f/0xf40 kthread+0x2a0/0x390 ret_from_fork+0x1f/0x40 Allocated by task 21948: kasan_kmalloc+0xa6/0xd0 kmem_cache_alloc_trace+0xdb/0x1c0 i40e_add_filter+0x11e/0x520 [i40e] i40e_addr_sync+0x37/0x60 [i40e] __hw_addr_sync_dev+0x1f5/0x2f0 i40e_set_rx_mode+0x61/0x1e0 [i40e] dev_uc_add_excl+0x137/0x190 i40e_ndo_fdb_add+0x161/0x260 [i40e] rtnl_fdb_add+0x567/0x950 rtnetlink_rcv_msg+0x5db/0x880 netlink_rcv_skb+0x254/0x380 netlink_unicast+0x454/0x610 netlink_sendmsg+0x747/0xb00 sock_sendmsg+0xe2/0x120 __sys_sendto+0x1ae/0x290 __x64_sys_sendto+0xdd/0x1b0 do_syscall_64+0xa0/0x370 entry_SYSCALL_64_after_hwframe+0x65/0xca Freed by task 21948: __kasan_slab_free+0x137/0x190 kfree+0x8b/0x1b0 __i40e_del_filter+0x116/0x1e0 [i40e] i40e_del_mac_filter+0x16c/0x300 [i40e] i40e_addr_unsync+0x134/0x1b0 [i40e] __hw_addr_sync_dev+0xff/0x2f0 i40e_set_rx_mode+0x61/0x1e0 [i40e] dev_uc_del+0x77/0x90 rtnl_fdb_del+0x6a5/0x860 rtnetlink_rcv_msg+0x5db/0x880 netlink_rcv_skb+0x254/0x380 netlink_unicast+0x454/0x610 netlink_sendmsg+0x747/0xb00 sock_sendmsg+0xe2/0x120 __sys_sendto+0x1ae/0x290 __x64_sys_sendto+0xdd/0x1b0 do_syscall_64+0xa0/0x370 entry_SYSCALL_64_after_hwframe+0x65/0xca Fixes: 3116f59c12bd ("i40e: fix use-after-free in i40e_sync_filters_subtask()") Fixes: 41c445ff0f48 ("i40e: main driver core") Signed-off-by: Ke Xiao <xiaoke@sangfor.com.cn> Signed-off-by: Ding Hui <dinghui@sangfor.com.cn> Cc: Di Zhu <zhudi2@huawei.com> Reviewed-by: Jan Sokolowski <jan.sokolowski@intel.com> Reviewed-by: Simon Horman <horms@kernel.org> --- v2: - Order local variable declarations in Reverse Christmas Tree (RCT) --- drivers/net/ethernet/intel/i40e/i40e_main.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)