Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 5 Dec 2018 14:23:01 +0000 (UTC)
From:      Slava Shwartsman <slavash@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r341582 - head/sys/dev/mlx5/mlx5_en
Message-ID:  <201812051423.wB5EN1m0004265@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: slavash
Date: Wed Dec  5 14:23:01 2018
New Revision: 341582
URL: https://svnweb.freebsd.org/changeset/base/341582

Log:
  mlx5en: Fix race in mlx5e_ethtool_debug_stats().
  
  Writing to the debug stats variable must be locked,
  else serialization will be lost which might cause
  various kernel panics due to creating and destroying
  sysctls out of order.
  
  Make sure the sysctl context is initialized after freeing
  the sysctl nodes, else they can be freed twice.
  
  Submitted by:   hselasky@
  Approved by:    hselasky (mentor)
  MFC after:      1 week
  Sponsored by:   Mellanox Technologies

Modified:
  head/sys/dev/mlx5/mlx5_en/en.h
  head/sys/dev/mlx5/mlx5_en/mlx5_en_ethtool.c
  head/sys/dev/mlx5/mlx5_en/mlx5_en_main.c

Modified: head/sys/dev/mlx5/mlx5_en/en.h
==============================================================================
--- head/sys/dev/mlx5/mlx5_en/en.h	Wed Dec  5 14:22:30 2018	(r341581)
+++ head/sys/dev/mlx5/mlx5_en/en.h	Wed Dec  5 14:23:01 2018	(r341582)
@@ -794,7 +794,6 @@ struct mlx5e_priv {
 	struct sysctl_oid *sysctl_hw;
 	int	sysctl_debug;
 	struct mlx5e_stats stats;
-	struct sysctl_ctx_list sysctl_ctx_channel_debug;
 	int	counter_set_id;
 
 	struct workqueue_struct *wq;

Modified: head/sys/dev/mlx5/mlx5_en/mlx5_en_ethtool.c
==============================================================================
--- head/sys/dev/mlx5/mlx5_en/mlx5_en_ethtool.c	Wed Dec  5 14:22:30 2018	(r341581)
+++ head/sys/dev/mlx5/mlx5_en/mlx5_en_ethtool.c	Wed Dec  5 14:23:01 2018	(r341582)
@@ -1056,33 +1056,34 @@ static int
 mlx5e_ethtool_debug_stats(SYSCTL_HANDLER_ARGS)
 {
 	struct mlx5e_priv *priv = arg1;
-	int error, sys_debug;
+	int sys_debug;
+	int error;
 
+	PRIV_LOCK(priv);
 	sys_debug = priv->sysctl_debug;
-	error = sysctl_handle_int(oidp, &priv->sysctl_debug, 0, req);
+	error = sysctl_handle_int(oidp, &sys_debug, 0, req);
 	if (error != 0 || !req->newptr)
-		return (error);
-	priv->sysctl_debug = priv->sysctl_debug != 0;
+		goto done;
+	sys_debug = sys_debug ? 1 : 0;
 	if (sys_debug == priv->sysctl_debug)
-		return (0);
+		goto done;
 
-	PRIV_LOCK(priv);
-	if (priv->sysctl_debug) {
+	if ((priv->sysctl_debug = sys_debug)) {
 		mlx5e_create_stats(&priv->stats.port_stats_debug.ctx,
 		    SYSCTL_CHILDREN(priv->sysctl_ifnet), "debug_stats",
 		    mlx5e_port_stats_debug_desc, MLX5E_PORT_STATS_DEBUG_NUM,
 		    priv->stats.port_stats_debug.arg);
-		SYSCTL_ADD_PROC(&priv->sysctl_ctx_channel_debug,
+		SYSCTL_ADD_PROC(&priv->stats.port_stats_debug.ctx,
 		    SYSCTL_CHILDREN(priv->sysctl_ifnet), OID_AUTO,
 		    "hw_ctx_debug",
 		    CTLFLAG_RD | CTLFLAG_MPSAFE | CTLTYPE_STRING, priv, 0,
 		    mlx5e_ethtool_debug_channel_info, "S", "");
 	} else {
 		sysctl_ctx_free(&priv->stats.port_stats_debug.ctx);
-		sysctl_ctx_free(&priv->sysctl_ctx_channel_debug);
 	}
+done:
 	PRIV_UNLOCK(priv);
-	return (0);
+	return (error);
 }
 
 static void

Modified: head/sys/dev/mlx5/mlx5_en/mlx5_en_main.c
==============================================================================
--- head/sys/dev/mlx5/mlx5_en/mlx5_en_main.c	Wed Dec  5 14:22:30 2018	(r341581)
+++ head/sys/dev/mlx5/mlx5_en/mlx5_en_main.c	Wed Dec  5 14:23:01 2018	(r341582)
@@ -3639,8 +3639,6 @@ mlx5e_create_ifp(struct mlx5_core_dev *mdev)
 	if (ifp->if_capenable & IFCAP_TXCSUM_IPV6)
 		ifp->if_hwassist |= (CSUM_UDP_IPV6 | CSUM_TCP_IPV6);
 
-	sysctl_ctx_init(&priv->sysctl_ctx_channel_debug);
-
 	/* ifnet sysctl tree */
 	sysctl_ctx_init(&priv->sysctl_ctx);
 	priv->sysctl_ifnet = SYSCTL_ADD_NODE(&priv->sysctl_ctx, SYSCTL_STATIC_CHILDREN(_dev),
@@ -3831,8 +3829,8 @@ err_free_wq:
 
 err_free_sysctl:
 	sysctl_ctx_free(&priv->sysctl_ctx);
-	sysctl_ctx_free(&priv->sysctl_ctx_channel_debug);
-
+	if (priv->sysctl_debug)
+		sysctl_ctx_free(&priv->stats.port_stats_debug.ctx);
 	if_free(ifp);
 
 err_free_priv:
@@ -3889,13 +3887,11 @@ mlx5e_destroy_ifp(struct mlx5_core_dev *mdev, void *vp
 	mlx5e_rl_cleanup(priv);
 #endif
 	/* destroy all remaining sysctl nodes */
-	if (priv->sysctl_debug) {
-		sysctl_ctx_free(&priv->sysctl_ctx_channel_debug);
-		sysctl_ctx_free(&priv->stats.port_stats_debug.ctx);
-	}
 	sysctl_ctx_free(&priv->stats.vport.ctx);
 	sysctl_ctx_free(&priv->stats.pport.ctx);
 	sysctl_ctx_free(&priv->sysctl_ctx);
+	if (priv->sysctl_debug)
+		sysctl_ctx_free(&priv->stats.port_stats_debug.ctx);
 
 	mlx5_core_destroy_mkey(priv->mdev, &priv->mr);
 	mlx5_dealloc_transport_domain(priv->mdev, priv->tdn);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201812051423.wB5EN1m0004265>