All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brett Creeley <bcreeley@amd.com>
To: Ke Xiao <xiaoke@sangfor.com.cn>,
	jesse.brandeburg@intel.com, anthony.l.nguyen@intel.com,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com
Cc: intel-wired-lan@lists.osuosl.org, zhudi2@huawei.com,
	dinghui@sangfor.com.cn, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [Intel-wired-lan] [net PATCH] i40e: fix use-after-free in i40e_aqc_add_filters()
Date: Fri, 15 Dec 2023 09:16:00 -0800	[thread overview]
Message-ID: <0edc953a-0357-d054-d9a2-e9a86e90233d@amd.com> (raw)
In-Reply-To: <20231213104912.16153-1-xiaoke@sangfor.com.cn>

On 12/13/2023 2:49 AM, Ke Xiao wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> 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>
> ---
>   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..16b574d69843 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -108,11 +108,17 @@ static void netdev_hw_addr_refcnt(struct i40e_mac_filter *f,
>                                    struct net_device *netdev, int delta)
>   {
>          struct netdev_hw_addr *ha;
> +       struct netdev_hw_addr_list *ha_list;

Nit, needs to be in Reverse Christmas Tree (RCT) order.
> 
>          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)
> --
> 2.17.1
> 
> 
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

WARNING: multiple messages have this Message-ID (diff)
From: Brett Creeley <bcreeley@amd.com>
To: Ke Xiao <xiaoke@sangfor.com.cn>,
	jesse.brandeburg@intel.com, anthony.l.nguyen@intel.com,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com
Cc: dinghui@sangfor.com.cn, zhudi2@huawei.com,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [net PATCH] i40e: fix use-after-free in i40e_aqc_add_filters()
Date: Fri, 15 Dec 2023 09:16:00 -0800	[thread overview]
Message-ID: <0edc953a-0357-d054-d9a2-e9a86e90233d@amd.com> (raw)
In-Reply-To: <20231213104912.16153-1-xiaoke@sangfor.com.cn>

On 12/13/2023 2:49 AM, Ke Xiao wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> 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>
> ---
>   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..16b574d69843 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -108,11 +108,17 @@ static void netdev_hw_addr_refcnt(struct i40e_mac_filter *f,
>                                    struct net_device *netdev, int delta)
>   {
>          struct netdev_hw_addr *ha;
> +       struct netdev_hw_addr_list *ha_list;

Nit, needs to be in Reverse Christmas Tree (RCT) order.
> 
>          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)
> --
> 2.17.1
> 
> 

  parent reply	other threads:[~2023-12-15 17:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-13 10:49 [Intel-wired-lan] [net PATCH] i40e: fix use-after-free in i40e_aqc_add_filters() Ke Xiao
2023-12-13 10:49 ` Ke Xiao
2023-12-13 13:24 ` [Intel-wired-lan] " Sokolowski, Jan
2023-12-13 13:24   ` Sokolowski, Jan
2023-12-15 16:28 ` [Intel-wired-lan] " Simon Horman
2023-12-15 16:28   ` Simon Horman
2023-12-15 17:16 ` Brett Creeley [this message]
2023-12-15 17:16   ` Brett Creeley
2023-12-18  6:54   ` [Intel-wired-lan] " xiaoke
2023-12-18  6:54     ` xiaoke

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0edc953a-0357-d054-d9a2-e9a86e90233d@amd.com \
    --to=bcreeley@amd.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=dinghui@sangfor.com.cn \
    --cc=edumazet@google.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jesse.brandeburg@intel.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=xiaoke@sangfor.com.cn \
    --cc=zhudi2@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.