From patchwork Thu Mar 8 22:52:07 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Kirsher, Jeffrey T" X-Patchwork-Id: 883393 X-Patchwork-Delegate: jeffrey.t.kirsher@intel.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=osuosl.org (client-ip=140.211.166.138; helo=whitealder.osuosl.org; envelope-from=intel-wired-lan-bounces@osuosl.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=intel.com Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3zy5P15wZVz9scR for ; Fri, 9 Mar 2018 09:52:05 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 705058936D; Thu, 8 Mar 2018 22:52:04 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Sa5MRAoQiyyE; Thu, 8 Mar 2018 22:52:03 +0000 (UTC) Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by whitealder.osuosl.org (Postfix) with ESMTP id B80CE89368; Thu, 8 Mar 2018 22:52:02 +0000 (UTC) X-Original-To: intel-wired-lan@lists.osuosl.org Delivered-To: intel-wired-lan@lists.osuosl.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by ash.osuosl.org (Postfix) with ESMTP id EB0231CF0D0 for ; Thu, 8 Mar 2018 22:51:53 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id E5B8122688 for ; Thu, 8 Mar 2018 22:51:53 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id PoVqRwSu8TF1 for ; Thu, 8 Mar 2018 22:51:51 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by silver.osuosl.org (Postfix) with ESMTPS id 90C832736E for ; Thu, 8 Mar 2018 22:51:47 +0000 (UTC) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Mar 2018 14:51:46 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.47,442,1515484800"; d="scan'208";a="37164039" Received: from jtkirshe-nuc.jf.intel.com ([134.134.177.59]) by orsmga001.jf.intel.com with ESMTP; 08 Mar 2018 14:51:45 -0800 From: Jeff Kirsher To: intel-wired-lan@lists.osuosl.org Date: Thu, 8 Mar 2018 14:52:07 -0800 Message-Id: <20180308225211.12254-3-jeffrey.t.kirsher@intel.com> X-Mailer: git-send-email 2.14.3 In-Reply-To: <20180308225211.12254-1-jeffrey.t.kirsher@intel.com> References: <20180308225211.12254-1-jeffrey.t.kirsher@intel.com> Subject: [Intel-wired-lan] [S87 v5 3/7] i40e: Fix permission check for VF MAC filters X-BeenThere: intel-wired-lan@osuosl.org X-Mailman-Version: 2.1.24 Precedence: list List-Id: Intel Wired Ethernet Linux Kernel Driver Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-wired-lan-bounces@osuosl.org Sender: "Intel-wired-lan" From: Filip Sadowski When VF requests adding of MAC filters the checking is done against number of already present MAC filters not adding them at the same time. It makes it possible to add a bunch of filters at once possibly exceeding acceptable limit of I40E_VC_MAX_MAC_ADDR_PER_VF filters. This happens because when checking vf->num_mac, we do not check how many filters are being requested at once. Modify the check function to ensure that it knows how many filters are being requested. This allows the check to ensure that the total number of filters in a single request does not cause us to go over the limit. Additionally, move the check to within the lock to ensure that the vf->num_mac is checked while holding the lock to maintain consistency. We could have simply moved the call to i40e_vf_check_permission to within the loop, but this could cause a request to be non-atomic, and add some but not all the addresses, while reporting an error code. We want to avoid this behavior so that users are not confused about which filters have or have not been added. Signed-off-by: Filip Sadowski Tested-by: Andrew Bowers --- drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 79 ++++++++++++++-------- 1 file changed, 51 insertions(+), 28 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c index 8559c099525d..8d9a4568ca24 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c @@ -2369,25 +2369,47 @@ static int i40e_vc_get_stats_msg(struct i40e_vf *vf, u8 *msg, u16 msglen) /** * i40e_check_vf_permission * @vf: pointer to the VF info - * @macaddr: pointer to the MAC Address being checked + * @al: MAC address list from virtchnl * - * Check if the VF has permission to add or delete unicast MAC address - * filters and return error code -EPERM if not. Then check if the - * address filter requested is broadcast or zero and if so return - * an invalid MAC address error code. + * Check that the given list of MAC addresses is allowed. Will return -EPERM + * if any address in the list is not valid. Checks the following conditions: + * + * 1) broadcast and zero addresses are never valid + * 2) unicast addresses are not allowed if the VMM has administratively set + * the VF MAC address, unless the VF is marked as privileged. + * 3) There is enough space to add all the addresses. + * + * Note that to guarantee consistency, it is expected this function be called + * while holding the mac_filter_hash_lock, as otherwise the current number of + * addresses might not be accurate. **/ -static inline int i40e_check_vf_permission(struct i40e_vf *vf, u8 *macaddr) +static inline int i40e_check_vf_permission(struct i40e_vf *vf, + struct virtchnl_ether_addr_list *al) { struct i40e_pf *pf = vf->pf; - int ret = 0; + int i; + + /* If this VF is not privileged, then we can't add more than a limited + * number of addresses. Check to make sure that the additions do not + * push us over the limit. + */ + if (!test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps) && + (vf->num_mac + al->num_elements) > I40E_VC_MAX_MAC_ADDR_PER_VF) { + dev_err(&pf->pdev->dev, + "Cannot add more MAC addresses, VF is not trusted, switch the VF to trusted to add more functionality\n"); + return -EPERM; + } + + for (i = 0; i < al->num_elements; i++) { + u8 *addr = al->list[i].addr; + + if (is_broadcast_ether_addr(addr) || + is_zero_ether_addr(addr)) { + dev_err(&pf->pdev->dev, "invalid VF MAC addr %pM\n", + addr); + return I40E_ERR_INVALID_MAC_ADDR; + } - if (is_broadcast_ether_addr(macaddr) || - is_zero_ether_addr(macaddr)) { - dev_err(&pf->pdev->dev, "invalid VF MAC addr %pM\n", macaddr); - ret = I40E_ERR_INVALID_MAC_ADDR; - } else if (vf->pf_set_mac && !is_multicast_ether_addr(macaddr) && - !test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps) && - !ether_addr_equal(macaddr, vf->default_lan_addr.addr)) { /* If the host VMM administrator has set the VF MAC address * administratively via the ndo_set_vf_mac command then deny * permission to the VF to add or delete unicast MAC addresses. @@ -2395,16 +2417,16 @@ static inline int i40e_check_vf_permission(struct i40e_vf *vf, u8 *macaddr) * The VF may request to set the MAC address filter already * assigned to it so do not return an error in that case. */ - dev_err(&pf->pdev->dev, - "VF attempting to override administratively set MAC address, reload the VF driver to resume normal operation\n"); - ret = -EPERM; - } else if ((vf->num_mac >= I40E_VC_MAX_MAC_ADDR_PER_VF) && - !test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps)) { - dev_err(&pf->pdev->dev, - "VF is not trusted, switch the VF to trusted to add more functionality\n"); - ret = -EPERM; + if (!test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps) && + !is_multicast_ether_addr(addr) && vf->pf_set_mac && + !ether_addr_equal(addr, vf->default_lan_addr.addr)) { + dev_err(&pf->pdev->dev, + "VF attempting to override administratively set MAC address, reload the VF driver to resume normal operation\n"); + return -EPERM; + } } - return ret; + + return 0; } /** @@ -2431,11 +2453,6 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen) goto error_param; } - for (i = 0; i < al->num_elements; i++) { - ret = i40e_check_vf_permission(vf, al->list[i].addr); - if (ret) - goto error_param; - } vsi = pf->vsi[vf->lan_vsi_idx]; /* Lock once, because all function inside for loop accesses VSI's @@ -2443,6 +2460,12 @@ static int i40e_vc_add_mac_addr_msg(struct i40e_vf *vf, u8 *msg, u16 msglen) */ spin_lock_bh(&vsi->mac_filter_hash_lock); + ret = i40e_check_vf_permission(vf, al); + if (ret) { + spin_unlock_bh(&vsi->mac_filter_hash_lock); + goto error_param; + } + /* add new addresses to the list */ for (i = 0; i < al->num_elements; i++) { struct i40e_mac_filter *f;