Commit 91fc6177 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files
Tony Nguyen says:

====================
i40e: virtchnl improvements

Przemek Kitszel says:

Improvements hardening PF-VF communication for i40e driver.
This patchset targets several issues that can cause undefined behavior
or be exploited in some other way.

* '40GbE' of git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue:
  i40e: improve VF MAC filters accounting
  i40e: add mask to apply valid bits for itr_idx
  i40e: add max boundary check for VF filters
  i40e: fix validation of VF state in get resources
  i40e: fix input validation logic for action_meta
  i40e: fix idx validation in config queues msg
  i40e: fix idx validation in i40e_validate_queue_map
  i40e: add validation for ring_len param
====================

Link: https://patch.msgid.link/20250919184959.656681-1-anthony.l.nguyen@intel.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 3491bb7d b99dd770
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -1278,7 +1278,8 @@ struct i40e_mac_filter *i40e_add_mac_filter(struct i40e_vsi *vsi,
					    const u8 *macaddr);
int i40e_del_mac_filter(struct i40e_vsi *vsi, const u8 *macaddr);
bool i40e_is_vsi_in_vlan(struct i40e_vsi *vsi);
int i40e_count_filters(struct i40e_vsi *vsi);
int i40e_count_all_filters(struct i40e_vsi *vsi);
int i40e_count_active_filters(struct i40e_vsi *vsi);
struct i40e_mac_filter *i40e_find_mac(struct i40e_vsi *vsi, const u8 *macaddr);
void i40e_vlan_stripping_enable(struct i40e_vsi *vsi);
static inline bool i40e_is_sw_dcb(struct i40e_pf *pf)
+22 −4
Original line number Diff line number Diff line
@@ -1243,12 +1243,30 @@ void i40e_update_stats(struct i40e_vsi *vsi)
}

/**
 * i40e_count_filters - counts VSI mac filters
 * i40e_count_all_filters - counts VSI MAC filters
 * @vsi: the VSI to be searched
 *
 * Returns count of mac filters
 **/
int i40e_count_filters(struct i40e_vsi *vsi)
 * Return: count of MAC filters in any state.
 */
int i40e_count_all_filters(struct i40e_vsi *vsi)
{
	struct i40e_mac_filter *f;
	struct hlist_node *h;
	int bkt, cnt = 0;

	hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist)
		cnt++;

	return cnt;
}

/**
 * i40e_count_active_filters - counts VSI MAC filters
 * @vsi: the VSI to be searched
 *
 * Return: count of active MAC filters.
 */
int i40e_count_active_filters(struct i40e_vsi *vsi)
{
	struct i40e_mac_filter *f;
	struct hlist_node *h;
+64 −46
Original line number Diff line number Diff line
@@ -448,7 +448,7 @@ static void i40e_config_irq_link_list(struct i40e_vf *vf, u16 vsi_id,
		    (qtype << I40E_QINT_RQCTL_NEXTQ_TYPE_SHIFT) |
		    (pf_queue_id << I40E_QINT_RQCTL_NEXTQ_INDX_SHIFT) |
		    BIT(I40E_QINT_RQCTL_CAUSE_ENA_SHIFT) |
		    (itr_idx << I40E_QINT_RQCTL_ITR_INDX_SHIFT);
		    FIELD_PREP(I40E_QINT_RQCTL_ITR_INDX_MASK, itr_idx);
		wr32(hw, reg_idx, reg);
	}

@@ -653,6 +653,13 @@ static int i40e_config_vsi_tx_queue(struct i40e_vf *vf, u16 vsi_id,

	/* only set the required fields */
	tx_ctx.base = info->dma_ring_addr / 128;

	/* ring_len has to be multiple of 8 */
	if (!IS_ALIGNED(info->ring_len, 8) ||
	    info->ring_len > I40E_MAX_NUM_DESCRIPTORS_XL710) {
		ret = -EINVAL;
		goto error_context;
	}
	tx_ctx.qlen = info->ring_len;
	tx_ctx.rdylist = le16_to_cpu(vsi->info.qs_handle[0]);
	tx_ctx.rdylist_act = 0;
@@ -716,6 +723,13 @@ static int i40e_config_vsi_rx_queue(struct i40e_vf *vf, u16 vsi_id,

	/* only set the required fields */
	rx_ctx.base = info->dma_ring_addr / 128;

	/* ring_len has to be multiple of 32 */
	if (!IS_ALIGNED(info->ring_len, 32) ||
	    info->ring_len > I40E_MAX_NUM_DESCRIPTORS_XL710) {
		ret = -EINVAL;
		goto error_param;
	}
	rx_ctx.qlen = info->ring_len;

	if (info->splithdr_enabled) {
@@ -1450,6 +1464,7 @@ static void i40e_trigger_vf_reset(struct i40e_vf *vf, bool flr)
	 * functions that may still be running at this point.
	 */
	clear_bit(I40E_VF_STATE_INIT, &vf->vf_states);
	clear_bit(I40E_VF_STATE_RESOURCES_LOADED, &vf->vf_states);

	/* In the case of a VFLR, the HW has already reset the VF and we
	 * just need to clean up, so don't hit the VFRTRIG register.
@@ -2116,7 +2131,10 @@ static int i40e_vc_get_vf_resources_msg(struct i40e_vf *vf, u8 *msg)
	size_t len = 0;
	int ret;

	if (!i40e_sync_vf_state(vf, I40E_VF_STATE_INIT)) {
	i40e_sync_vf_state(vf, I40E_VF_STATE_INIT);

	if (!test_bit(I40E_VF_STATE_INIT, &vf->vf_states) ||
	    test_bit(I40E_VF_STATE_RESOURCES_LOADED, &vf->vf_states)) {
		aq_ret = -EINVAL;
		goto err;
	}
@@ -2219,6 +2237,7 @@ static int i40e_vc_get_vf_resources_msg(struct i40e_vf *vf, u8 *msg)
				vf->default_lan_addr.addr);
	}
	set_bit(I40E_VF_STATE_ACTIVE, &vf->vf_states);
	set_bit(I40E_VF_STATE_RESOURCES_LOADED, &vf->vf_states);

err:
	/* send the response back to the VF */
@@ -2381,7 +2400,7 @@ static int i40e_vc_config_queues_msg(struct i40e_vf *vf, u8 *msg)
		}

		if (vf->adq_enabled) {
			if (idx >= ARRAY_SIZE(vf->ch)) {
			if (idx >= vf->num_tc) {
				aq_ret = -ENODEV;
				goto error_param;
			}
@@ -2402,7 +2421,7 @@ static int i40e_vc_config_queues_msg(struct i40e_vf *vf, u8 *msg)
		 * to its appropriate VSIs based on TC mapping
		 */
		if (vf->adq_enabled) {
			if (idx >= ARRAY_SIZE(vf->ch)) {
			if (idx >= vf->num_tc) {
				aq_ret = -ENODEV;
				goto error_param;
			}
@@ -2452,8 +2471,10 @@ static int i40e_validate_queue_map(struct i40e_vf *vf, u16 vsi_id,
	u16 vsi_queue_id, queue_id;

	for_each_set_bit(vsi_queue_id, &queuemap, I40E_MAX_VSI_QP) {
		if (vf->adq_enabled) {
			vsi_id = vf->ch[vsi_queue_id / I40E_MAX_VF_VSI].vsi_id;
		u16 idx = vsi_queue_id / I40E_MAX_VF_VSI;

		if (vf->adq_enabled && idx < vf->num_tc) {
			vsi_id = vf->ch[idx].vsi_id;
			queue_id = (vsi_queue_id % I40E_DEFAULT_QUEUES_PER_VF);
		} else {
			queue_id = vsi_queue_id;
@@ -2841,24 +2862,6 @@ static int i40e_vc_get_stats_msg(struct i40e_vf *vf, u8 *msg)
				      (u8 *)&stats, sizeof(stats));
}

/**
 * i40e_can_vf_change_mac
 * @vf: pointer to the VF info
 *
 * Return true if the VF is allowed to change its MAC filters, false otherwise
 */
static bool i40e_can_vf_change_mac(struct i40e_vf *vf)
{
	/* If the VF MAC address has been set administratively (via the
	 * ndo_set_vf_mac command), then deny permission to the VF to
	 * add/delete unicast MAC addresses, unless the VF is trusted
	 */
	if (vf->pf_set_mac && !vf->trusted)
		return false;

	return true;
}

#define I40E_MAX_MACVLAN_PER_HW 3072
#define I40E_MAX_MACVLAN_PER_PF(num_ports) (I40E_MAX_MACVLAN_PER_HW /	\
	(num_ports))
@@ -2897,8 +2900,10 @@ static inline int i40e_check_vf_permission(struct i40e_vf *vf,
	struct i40e_pf *pf = vf->pf;
	struct i40e_vsi *vsi = pf->vsi[vf->lan_vsi_idx];
	struct i40e_hw *hw = &pf->hw;
	int mac2add_cnt = 0;
	int i;
	int i, mac_add_max, mac_add_cnt = 0;
	bool vf_trusted;

	vf_trusted = test_bit(I40E_VIRTCHNL_VF_CAP_PRIVILEGE, &vf->vf_caps);

	for (i = 0; i < al->num_elements; i++) {
		struct i40e_mac_filter *f;
@@ -2918,9 +2923,8 @@ static inline int i40e_check_vf_permission(struct i40e_vf *vf,
		 * The VF may request to set the MAC address filter already
		 * assigned to it so do not return an error in that case.
		 */
		if (!i40e_can_vf_change_mac(vf) &&
		    !is_multicast_ether_addr(addr) &&
		    !ether_addr_equal(addr, vf->default_lan_addr.addr)) {
		if (!vf_trusted && !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, bring down and up the VF interface to resume normal operation\n");
			return -EPERM;
@@ -2929,29 +2933,33 @@ static inline int i40e_check_vf_permission(struct i40e_vf *vf,
		/*count filters that really will be added*/
		f = i40e_find_mac(vsi, addr);
		if (!f)
			++mac2add_cnt;
			++mac_add_cnt;
	}

	/* 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)) {
		if ((i40e_count_filters(vsi) + mac2add_cnt) >
		    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;
		}
	/* If this VF is trusted, it can use more resources than untrusted.
	 * number of addresses.
	 *
	 * If this VF is trusted, it can use more resources than untrusted.
	 * However to ensure that every trusted VF has appropriate number of
	 * resources, divide whole pool of resources per port and then across
	 * all VFs.
	 */
	if (!vf_trusted)
		mac_add_max = I40E_VC_MAX_MAC_ADDR_PER_VF;
	else
		mac_add_max = I40E_VC_MAX_MACVLAN_PER_TRUSTED_VF(pf->num_alloc_vfs, hw->num_ports);

	/* VF can replace all its filters in one step, in this case mac_add_max
	 * will be added as active and another mac_add_max will be in
	 * a to-be-removed state. Account for that.
	 */
	if ((i40e_count_active_filters(vsi) + mac_add_cnt) > mac_add_max ||
	    (i40e_count_all_filters(vsi) + mac_add_cnt) > 2 * mac_add_max) {
		if (!vf_trusted) {
			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;
		} else {
		if ((i40e_count_filters(vsi) + mac2add_cnt) >
		    I40E_VC_MAX_MACVLAN_PER_TRUSTED_VF(pf->num_alloc_vfs,
						       hw->num_ports)) {
			dev_err(&pf->pdev->dev,
				"Cannot add more MAC addresses, trusted VF exhausted it's resources\n");
			return -EPERM;
@@ -3587,7 +3595,7 @@ static int i40e_validate_cloud_filter(struct i40e_vf *vf,

	/* action_meta is TC number here to which the filter is applied */
	if (!tc_filter->action_meta ||
	    tc_filter->action_meta > vf->num_tc) {
	    tc_filter->action_meta >= vf->num_tc) {
		dev_info(&pf->pdev->dev, "VF %d: Invalid TC number %u\n",
			 vf->vf_id, tc_filter->action_meta);
		goto err;
@@ -3884,6 +3892,8 @@ static int i40e_vc_del_cloud_filter(struct i40e_vf *vf, u8 *msg)
				       aq_ret);
}

#define I40E_MAX_VF_CLOUD_FILTER 0xFF00

/**
 * i40e_vc_add_cloud_filter
 * @vf: pointer to the VF info
@@ -3923,6 +3933,14 @@ static int i40e_vc_add_cloud_filter(struct i40e_vf *vf, u8 *msg)
		goto err_out;
	}

	if (vf->num_cloud_filters >= I40E_MAX_VF_CLOUD_FILTER) {
		dev_warn(&pf->pdev->dev,
			 "VF %d: Max number of filters reached, can't apply cloud filter\n",
			 vf->vf_id);
		aq_ret = -ENOSPC;
		goto err_out;
	}

	cfilter = kzalloc(sizeof(*cfilter), GFP_KERNEL);
	if (!cfilter) {
		aq_ret = -ENOMEM;
+2 −1
Original line number Diff line number Diff line
@@ -41,7 +41,8 @@ enum i40e_vf_states {
	I40E_VF_STATE_MC_PROMISC,
	I40E_VF_STATE_UC_PROMISC,
	I40E_VF_STATE_PRE_ENABLE,
	I40E_VF_STATE_RESETTING
	I40E_VF_STATE_RESETTING,
	I40E_VF_STATE_RESOURCES_LOADED,
};

/* VF capabilities */