lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 27 Mar 2018 17:44:36 +0100
From:   Edward Cree <ecree@...arflare.com>
To:     <linux-net-drivers@...arflare.com>,
        David Miller <davem@...emloft.net>
CC:     netdev <netdev@...r.kernel.org>
Subject: [PATCH net-next 5/6] sfc: protect list of RSS contexts under a mutex

Otherwise races are possible between ethtool ops and
 efx_ef10_rx_restore_rss_contexts().
Also, don't try to perform the restore on every reset, only after an MC
 reboot, otherwise we'll leak RSS contexts on the NIC.

Fixes: 42356d9a137b ("sfc: support RSS spreading of ethtool ntuple filters")
Signed-off-by: Edward Cree <ecree@...arflare.com>
---
 drivers/net/ethernet/sfc/ef10.c       | 32 ++++++++++++++---
 drivers/net/ethernet/sfc/efx.c        | 14 ++++++--
 drivers/net/ethernet/sfc/efx.h        |  4 +--
 drivers/net/ethernet/sfc/ethtool.c    | 66 +++++++++++++++++++++++------------
 drivers/net/ethernet/sfc/net_driver.h |  2 ++
 drivers/net/ethernet/sfc/nic.h        |  3 ++
 6 files changed, 89 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index 9a139c390037..c4c45c94da77 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -1499,6 +1499,7 @@ static void efx_ef10_reset_mc_allocations(struct efx_nic *efx)
 
 	/* All our allocations have been reset */
 	nic_data->must_realloc_vis = true;
+	nic_data->must_restore_rss_contexts = true;
 	nic_data->must_restore_filters = true;
 	nic_data->must_restore_piobufs = true;
 	efx_ef10_forget_old_piobufs(efx);
@@ -2899,6 +2900,8 @@ static int efx_ef10_rx_push_rss_context_config(struct efx_nic *efx,
 {
 	int rc;
 
+	WARN_ON(!mutex_is_locked(&efx->rss_lock));
+
 	if (ctx->context_id == EFX_EF10_RSS_CONTEXT_INVALID) {
 		rc = efx_ef10_alloc_rss_context(efx, true, ctx, NULL);
 		if (rc)
@@ -2929,6 +2932,8 @@ static int efx_ef10_rx_pull_rss_context_config(struct efx_nic *efx,
 	size_t outlen;
 	int rc, i;
 
+	WARN_ON(!mutex_is_locked(&efx->rss_lock));
+
 	BUILD_BUG_ON(MC_CMD_RSS_CONTEXT_GET_TABLE_IN_LEN !=
 		     MC_CMD_RSS_CONTEXT_GET_KEY_IN_LEN);
 
@@ -2972,14 +2977,25 @@ static int efx_ef10_rx_pull_rss_context_config(struct efx_nic *efx,
 
 static int efx_ef10_rx_pull_rss_config(struct efx_nic *efx)
 {
-	return efx_ef10_rx_pull_rss_context_config(efx, &efx->rss_context);
+	int rc;
+
+	mutex_lock(&efx->rss_lock);
+	rc = efx_ef10_rx_pull_rss_context_config(efx, &efx->rss_context);
+	mutex_unlock(&efx->rss_lock);
+	return rc;
 }
 
 static void efx_ef10_rx_restore_rss_contexts(struct efx_nic *efx)
 {
+	struct efx_ef10_nic_data *nic_data = efx->nic_data;
 	struct efx_rss_context *ctx;
 	int rc;
 
+	WARN_ON(!mutex_is_locked(&efx->rss_lock));
+
+	if (!nic_data->must_restore_rss_contexts)
+		return;
+
 	list_for_each_entry(ctx, &efx->rss_context.list, list) {
 		/* previous NIC RSS context is gone */
 		ctx->context_id = EFX_EF10_RSS_CONTEXT_INVALID;
@@ -2993,6 +3009,7 @@ static void efx_ef10_rx_restore_rss_contexts(struct efx_nic *efx)
 				   "; RSS filters may fail to be applied\n",
 				   ctx->user_id, rc);
 	}
+	nic_data->must_restore_rss_contexts = false;
 }
 
 static int efx_ef10_pf_rx_push_rss_config(struct efx_nic *efx, bool user,
@@ -4307,6 +4324,7 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx,
 	struct efx_rss_context *ctx = NULL;
 	unsigned int match_pri, hash;
 	unsigned int priv_flags;
+	bool rss_locked = false;
 	bool replacing = false;
 	unsigned int depth, i;
 	int ins_index = -1;
@@ -4336,9 +4354,10 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx,
 		bitmap_zero(mc_rem_map, EFX_EF10_FILTER_SEARCH_LIMIT);
 
 	if (spec->flags & EFX_FILTER_FLAG_RX_RSS) {
+		mutex_lock(&efx->rss_lock);
+		rss_locked = true;
 		if (spec->rss_context)
-			ctx = efx_find_rss_context_entry(spec->rss_context,
-							 &efx->rss_context.list);
+			ctx = efx_find_rss_context_entry(efx, spec->rss_context);
 		else
 			ctx = &efx->rss_context;
 		if (!ctx) {
@@ -4501,6 +4520,8 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx,
 		rc = efx_ef10_make_filter_id(match_pri, ins_index);
 
 out_unlock:
+	if (rss_locked)
+		mutex_unlock(&efx->rss_lock);
 	up_write(&table->lock);
 	up_read(&efx->filter_sem);
 	return rc;
@@ -5008,6 +5029,7 @@ static void efx_ef10_filter_table_restore(struct efx_nic *efx)
 		return;
 
 	down_write(&table->lock);
+	mutex_lock(&efx->rss_lock);
 
 	for (filter_idx = 0; filter_idx < HUNT_FILTER_TBL_ROWS; filter_idx++) {
 		spec = efx_ef10_filter_entry_spec(table, filter_idx);
@@ -5024,8 +5046,7 @@ static void efx_ef10_filter_table_restore(struct efx_nic *efx)
 			goto not_restored;
 		}
 		if (spec->rss_context)
-			ctx = efx_find_rss_context_entry(spec->rss_context,
-							 &efx->rss_context.list);
+			ctx = efx_find_rss_context_entry(efx, spec->rss_context);
 		else
 			ctx = &efx->rss_context;
 		if (spec->flags & EFX_FILTER_FLAG_RX_RSS) {
@@ -5064,6 +5085,7 @@ static void efx_ef10_filter_table_restore(struct efx_nic *efx)
 		}
 	}
 
+	mutex_unlock(&efx->rss_lock);
 	up_write(&table->lock);
 
 	/* This can happen validly if the MC's capabilities have changed, so
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index cb9273016916..692dd729ee2a 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -2657,6 +2657,7 @@ void efx_reset_down(struct efx_nic *efx, enum reset_type method)
 	efx_disable_interrupts(efx);
 
 	mutex_lock(&efx->mac_lock);
+	mutex_lock(&efx->rss_lock);
 	if (efx->port_initialized && method != RESET_TYPE_INVISIBLE &&
 	    method != RESET_TYPE_DATAPATH)
 		efx->phy_op->fini(efx);
@@ -2712,6 +2713,7 @@ int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok)
 
 	if (efx->type->rx_restore_rss_contexts)
 		efx->type->rx_restore_rss_contexts(efx);
+	mutex_unlock(&efx->rss_lock);
 	down_read(&efx->filter_sem);
 	efx_restore_filters(efx);
 	up_read(&efx->filter_sem);
@@ -2730,6 +2732,7 @@ int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok)
 fail:
 	efx->port_initialized = false;
 
+	mutex_unlock(&efx->rss_lock);
 	mutex_unlock(&efx->mac_lock);
 
 	return rc;
@@ -3016,6 +3019,7 @@ static int efx_init_struct(struct efx_nic *efx,
 	efx->rx_packet_ts_offset =
 		efx->type->rx_ts_offset - efx->type->rx_prefix_size;
 	INIT_LIST_HEAD(&efx->rss_context.list);
+	mutex_init(&efx->rss_lock);
 	spin_lock_init(&efx->stats_lock);
 	efx->vi_stride = EFX_DEFAULT_VI_STRIDE;
 	efx->num_mac_stats = MC_CMD_MAC_NSTATS;
@@ -3091,11 +3095,14 @@ void efx_update_sw_stats(struct efx_nic *efx, u64 *stats)
 /* RSS contexts.  We're using linked lists and crappy O(n) algorithms, because
  * (a) this is an infrequent control-plane operation and (b) n is small (max 64)
  */
-struct efx_rss_context *efx_alloc_rss_context_entry(struct list_head *head)
+struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx)
 {
+	struct list_head *head = &efx->rss_context.list;
 	struct efx_rss_context *ctx, *new;
 	u32 id = 1; /* Don't use zero, that refers to the master RSS context */
 
+	WARN_ON(!mutex_is_locked(&efx->rss_lock));
+
 	/* Search for first gap in the numbering */
 	list_for_each_entry(ctx, head, list) {
 		if (ctx->user_id != id)
@@ -3121,10 +3128,13 @@ struct efx_rss_context *efx_alloc_rss_context_entry(struct list_head *head)
 	return new;
 }
 
-struct efx_rss_context *efx_find_rss_context_entry(u32 id, struct list_head *head)
+struct efx_rss_context *efx_find_rss_context_entry(struct efx_nic *efx, u32 id)
 {
+	struct list_head *head = &efx->rss_context.list;
 	struct efx_rss_context *ctx;
 
+	WARN_ON(!mutex_is_locked(&efx->rss_lock));
+
 	list_for_each_entry(ctx, head, list)
 		if (ctx->user_id == id)
 			return ctx;
diff --git a/drivers/net/ethernet/sfc/efx.h b/drivers/net/ethernet/sfc/efx.h
index 545c2ea1622e..a3140e16fcef 100644
--- a/drivers/net/ethernet/sfc/efx.h
+++ b/drivers/net/ethernet/sfc/efx.h
@@ -187,8 +187,8 @@ static inline void efx_filter_rfs_expire(struct work_struct *data) {}
 bool efx_filter_is_mc_recipient(const struct efx_filter_spec *spec);
 
 /* RSS contexts */
-struct efx_rss_context *efx_alloc_rss_context_entry(struct list_head *list);
-struct efx_rss_context *efx_find_rss_context_entry(u32 id, struct list_head *list);
+struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx);
+struct efx_rss_context *efx_find_rss_context_entry(struct efx_nic *efx, u32 id);
 void efx_free_rss_context_entry(struct efx_rss_context *ctx);
 static inline bool efx_rss_active(struct efx_rss_context *ctx)
 {
diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c
index 64049e71e6e7..de1fac6cd34f 100644
--- a/drivers/net/ethernet/sfc/ethtool.c
+++ b/drivers/net/ethernet/sfc/ethtool.c
@@ -979,7 +979,7 @@ efx_ethtool_get_rxnfc(struct net_device *net_dev,
 {
 	struct efx_nic *efx = netdev_priv(net_dev);
 	u32 rss_context = 0;
-	s32 rc;
+	s32 rc = 0;
 
 	switch (info->cmd) {
 	case ETHTOOL_GRXRINGS:
@@ -989,15 +989,17 @@ efx_ethtool_get_rxnfc(struct net_device *net_dev,
 	case ETHTOOL_GRXFH: {
 		struct efx_rss_context *ctx = &efx->rss_context;
 
+		mutex_lock(&efx->rss_lock);
 		if (info->flow_type & FLOW_RSS && info->rss_context) {
-			ctx = efx_find_rss_context_entry(info->rss_context,
-							 &efx->rss_context.list);
-			if (!ctx)
-				return -ENOENT;
+			ctx = efx_find_rss_context_entry(efx, info->rss_context);
+			if (!ctx) {
+				rc = -ENOENT;
+				goto out_unlock;
+			}
 		}
 		info->data = 0;
 		if (!efx_rss_active(ctx)) /* No RSS */
-			return 0;
+			goto out_unlock;
 		switch (info->flow_type & ~FLOW_RSS) {
 		case UDP_V4_FLOW:
 			if (ctx->rx_hash_udp_4tuple)
@@ -1024,7 +1026,9 @@ efx_ethtool_get_rxnfc(struct net_device *net_dev,
 		default:
 			break;
 		}
-		return 0;
+out_unlock:
+		mutex_unlock(&efx->rss_lock);
+		return rc;
 	}
 
 	case ETHTOOL_GRXCLSRLCNT:
@@ -1366,16 +1370,20 @@ static int efx_ethtool_get_rxfh_context(struct net_device *net_dev, u32 *indir,
 {
 	struct efx_nic *efx = netdev_priv(net_dev);
 	struct efx_rss_context *ctx;
-	int rc;
+	int rc = 0;
 
 	if (!efx->type->rx_pull_rss_context_config)
 		return -EOPNOTSUPP;
-	ctx = efx_find_rss_context_entry(rss_context, &efx->rss_context.list);
-	if (!ctx)
-		return -ENOENT;
+
+	mutex_lock(&efx->rss_lock);
+	ctx = efx_find_rss_context_entry(efx, rss_context);
+	if (!ctx) {
+		rc = -ENOENT;
+		goto out_unlock;
+	}
 	rc = efx->type->rx_pull_rss_context_config(efx, ctx);
 	if (rc)
-		return rc;
+		goto out_unlock;
 
 	if (hfunc)
 		*hfunc = ETH_RSS_HASH_TOP;
@@ -1383,7 +1391,9 @@ static int efx_ethtool_get_rxfh_context(struct net_device *net_dev, u32 *indir,
 		memcpy(indir, ctx->rx_indir_table, sizeof(ctx->rx_indir_table));
 	if (key)
 		memcpy(key, ctx->rx_hash_key, efx->type->rx_hash_key_size);
-	return 0;
+out_unlock:
+	mutex_unlock(&efx->rss_lock);
+	return rc;
 }
 
 static int efx_ethtool_set_rxfh_context(struct net_device *net_dev,
@@ -1401,23 +1411,31 @@ static int efx_ethtool_set_rxfh_context(struct net_device *net_dev,
 	/* Hash function is Toeplitz, cannot be changed */
 	if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
 		return -EOPNOTSUPP;
+
+	mutex_lock(&efx->rss_lock);
+
 	if (*rss_context == ETH_RXFH_CONTEXT_ALLOC) {
-		if (delete)
+		if (delete) {
 			/* alloc + delete == Nothing to do */
-			return -EINVAL;
-		ctx = efx_alloc_rss_context_entry(&efx->rss_context.list);
-		if (!ctx)
-			return -ENOMEM;
+			rc = -EINVAL;
+			goto out_unlock;
+		}
+		ctx = efx_alloc_rss_context_entry(efx);
+		if (!ctx) {
+			rc = -ENOMEM;
+			goto out_unlock;
+		}
 		ctx->context_id = EFX_EF10_RSS_CONTEXT_INVALID;
 		/* Initialise indir table and key to defaults */
 		efx_set_default_rx_indir_table(efx, ctx);
 		netdev_rss_key_fill(ctx->rx_hash_key, sizeof(ctx->rx_hash_key));
 		allocated = true;
 	} else {
-		ctx = efx_find_rss_context_entry(*rss_context,
-						 &efx->rss_context.list);
-		if (!ctx)
-			return -ENOENT;
+		ctx = efx_find_rss_context_entry(efx, *rss_context);
+		if (!ctx) {
+			rc = -ENOENT;
+			goto out_unlock;
+		}
 	}
 
 	if (delete) {
@@ -1425,7 +1443,7 @@ static int efx_ethtool_set_rxfh_context(struct net_device *net_dev,
 		rc = efx->type->rx_push_rss_context_config(efx, ctx, NULL, NULL);
 		if (!rc)
 			efx_free_rss_context_entry(ctx);
-		return rc;
+		goto out_unlock;
 	}
 
 	if (!key)
@@ -1438,6 +1456,8 @@ static int efx_ethtool_set_rxfh_context(struct net_device *net_dev,
 		efx_free_rss_context_entry(ctx);
 	else
 		*rss_context = ctx->user_id;
+out_unlock:
+	mutex_unlock(&efx->rss_lock);
 	return rc;
 }
 
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index ef6fc931afb8..15edf98114dd 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -791,6 +791,7 @@ struct efx_rss_context {
  * @rx_scatter: Scatter mode enabled for receives
  * @rss_context: Main RSS context.  Its @list member is the head of the list of
  *	RSS contexts created by user requests
+ * @rss_lock: Protects custom RSS context software state in @rss_context.list
  * @int_error_count: Number of internal errors seen recently
  * @int_error_expire: Time at which error count will be expired
  * @irq_soft_enabled: Are IRQs soft-enabled? If not, IRQ handler will
@@ -933,6 +934,7 @@ struct efx_nic {
 	int rx_packet_ts_offset;
 	bool rx_scatter;
 	struct efx_rss_context rss_context;
+	struct mutex rss_lock;
 
 	unsigned int_error_count;
 	unsigned long int_error_expire;
diff --git a/drivers/net/ethernet/sfc/nic.h b/drivers/net/ethernet/sfc/nic.h
index ac59afad6e5c..5640034bda10 100644
--- a/drivers/net/ethernet/sfc/nic.h
+++ b/drivers/net/ethernet/sfc/nic.h
@@ -365,6 +365,8 @@ enum {
  * @vi_base: Absolute index of first VI in this function
  * @n_allocated_vis: Number of VIs allocated to this function
  * @must_realloc_vis: Flag: VIs have yet to be reallocated after MC reboot
+ * @must_restore_rss_contexts: Flag: RSS contexts have yet to be restored after
+ *	MC reboot
  * @must_restore_filters: Flag: filters have yet to be restored after MC reboot
  * @n_piobufs: Number of PIO buffers allocated to this function
  * @wc_membase: Base address of write-combining mapping of the memory BAR
@@ -407,6 +409,7 @@ struct efx_ef10_nic_data {
 	unsigned int vi_base;
 	unsigned int n_allocated_vis;
 	bool must_realloc_vis;
+	bool must_restore_rss_contexts;
 	bool must_restore_filters;
 	unsigned int n_piobufs;
 	void __iomem *wc_membase, *pio_write_base;

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ