Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 30 Nov 2018 07:07:09 +0000 (UTC)
From:      Andrew Rybchenko <arybchik@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r341305 - head/sys/dev/sfxge/common
Message-ID:  <201811300707.wAU779Um082614@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: arybchik
Date: Fri Nov 30 07:07:09 2018
New Revision: 341305
URL: https://svnweb.freebsd.org/changeset/base/341305

Log:
  sfxge(4): check buffer size for hash flags
  
  The efx_rx_scale_hash_flags_get interface is unsafe, as it does not
  have an argument for the size of the output buffer used to return
  the flags. While the only caller currently supplies a sufficiently
  large buffer, this should be checked at runtime to avoid writing
  past the end of the buffer.
  
  Submitted by:   Ivan Malov <ivan.malov at oktetlabs.ru>
  Sponsored by:   Solarflare Communications, Inc.
  Differential Revision:  https://reviews.freebsd.org/D18267

Modified:
  head/sys/dev/sfxge/common/efx.h
  head/sys/dev/sfxge/common/efx_annote.h   (contents, props changed)
  head/sys/dev/sfxge/common/efx_rx.c

Modified: head/sys/dev/sfxge/common/efx.h
==============================================================================
--- head/sys/dev/sfxge/common/efx.h	Fri Nov 30 07:06:58 2018	(r341304)
+++ head/sys/dev/sfxge/common/efx.h	Fri Nov 30 07:07:09 2018	(r341305)
@@ -2396,7 +2396,8 @@ extern	__checkReturn				efx_rc_t
 efx_rx_scale_hash_flags_get(
 	__in					efx_nic_t *enp,
 	__in					efx_rx_hash_alg_t hash_alg,
-	__inout_ecount(EFX_RX_HASH_NFLAGS)	unsigned int *flagsp,
+	__out_ecount_part(max_nflags, *nflagsp)	unsigned int *flagsp,
+	__in					unsigned int max_nflags,
 	__out					unsigned int *nflagsp);
 
 extern	__checkReturn	efx_rc_t

Modified: head/sys/dev/sfxge/common/efx_annote.h
==============================================================================
--- head/sys/dev/sfxge/common/efx_annote.h	Fri Nov 30 07:06:58 2018	(r341304)
+++ head/sys/dev/sfxge/common/efx_annote.h	Fri Nov 30 07:07:09 2018	(r341305)
@@ -61,6 +61,7 @@
 #define	__out_opt
 #define	__out_ecount(_n)
 #define	__out_ecount_opt(_n)
+#define	__out_ecount_part(_n, _l)
 #define	__out_bcount(_n)
 #define	__out_bcount_opt(_n)
 #define	__out_bcount_part(_n, _l)

Modified: head/sys/dev/sfxge/common/efx_rx.c
==============================================================================
--- head/sys/dev/sfxge/common/efx_rx.c	Fri Nov 30 07:06:58 2018	(r341304)
+++ head/sys/dev/sfxge/common/efx_rx.c	Fri Nov 30 07:07:09 2018	(r341305)
@@ -327,13 +327,12 @@ fail1:
 efx_rx_scale_hash_flags_get(
 	__in					efx_nic_t *enp,
 	__in					efx_rx_hash_alg_t hash_alg,
-	__inout_ecount(EFX_RX_HASH_NFLAGS)	unsigned int *flagsp,
+	__out_ecount_part(max_nflags, *nflagsp)	unsigned int *flagsp,
+	__in					unsigned int max_nflags,
 	__out					unsigned int *nflagsp)
 {
 	efx_nic_cfg_t *encp = &enp->en_nic_cfg;
-	boolean_t l4;
-	boolean_t additional_modes;
-	unsigned int *entryp = flagsp;
+	unsigned int nflags = 0;
 	efx_rc_t rc;
 
 	if (flagsp == NULL || nflagsp == NULL) {
@@ -342,56 +341,90 @@ efx_rx_scale_hash_flags_get(
 	}
 
 	if ((encp->enc_rx_scale_hash_alg_mask & (1U << hash_alg)) == 0) {
-		*nflagsp = 0;
-		return 0;
+		nflags = 0;
+		goto done;
 	}
 
-	l4 = encp->enc_rx_scale_l4_hash_supported;
-	additional_modes = encp->enc_rx_scale_additional_modes_supported;
-
-#define	LIST_FLAGS(_entryp, _class, _l4_hashing, _additional_modes)	\
-	do {								\
-		if (_l4_hashing) {					\
-			*(_entryp++) = EFX_RX_HASH(_class, 4TUPLE);	\
-									\
-			if (_additional_modes) {			\
-				*(_entryp++) =				\
-				    EFX_RX_HASH(_class, 2TUPLE_DST);	\
-				*(_entryp++) =				\
-				    EFX_RX_HASH(_class, 2TUPLE_SRC);	\
-			}						\
-		}							\
-									\
-		*(_entryp++) = EFX_RX_HASH(_class, 2TUPLE);		\
-									\
-		if (_additional_modes) {				\
-			*(_entryp++) = EFX_RX_HASH(_class, 1TUPLE_DST);	\
-			*(_entryp++) = EFX_RX_HASH(_class, 1TUPLE_SRC);	\
-		}							\
-									\
-		*(_entryp++) = EFX_RX_HASH(_class, DISABLE);		\
-									\
-		_NOTE(CONSTANTCONDITION)				\
+	/* Helper to add flags word to flags array without buffer overflow */
+#define	INSERT_FLAGS(_flags)			\
+	do {					\
+		if (nflags >= max_nflags) {	\
+			rc = E2BIG;		\
+			goto fail2;		\
+		}				\
+		*(flagsp + nflags) = (_flags);	\
+		nflags++;			\
+						\
+		_NOTE(CONSTANTCONDITION)	\
 	} while (B_FALSE)
 
-	LIST_FLAGS(entryp, IPV4_TCP, l4, additional_modes);
-	LIST_FLAGS(entryp, IPV6_TCP, l4, additional_modes);
+	if (encp->enc_rx_scale_l4_hash_supported != B_FALSE) {
+		INSERT_FLAGS(EFX_RX_HASH(IPV4_TCP, 4TUPLE));
+		INSERT_FLAGS(EFX_RX_HASH(IPV6_TCP, 4TUPLE));
+	}
 
-	if (additional_modes) {
-		LIST_FLAGS(entryp, IPV4_UDP, l4, additional_modes);
-		LIST_FLAGS(entryp, IPV6_UDP, l4, additional_modes);
+	if ((encp->enc_rx_scale_l4_hash_supported != B_FALSE) &&
+	    (encp->enc_rx_scale_additional_modes_supported != B_FALSE)) {
+		INSERT_FLAGS(EFX_RX_HASH(IPV4_TCP, 2TUPLE_DST));
+		INSERT_FLAGS(EFX_RX_HASH(IPV4_TCP, 2TUPLE_SRC));
+
+		INSERT_FLAGS(EFX_RX_HASH(IPV6_TCP, 2TUPLE_DST));
+		INSERT_FLAGS(EFX_RX_HASH(IPV6_TCP, 2TUPLE_SRC));
+
+		INSERT_FLAGS(EFX_RX_HASH(IPV4_UDP, 4TUPLE));
+		INSERT_FLAGS(EFX_RX_HASH(IPV4_UDP, 2TUPLE_DST));
+		INSERT_FLAGS(EFX_RX_HASH(IPV4_UDP, 2TUPLE_SRC));
+
+		INSERT_FLAGS(EFX_RX_HASH(IPV6_UDP, 4TUPLE));
+		INSERT_FLAGS(EFX_RX_HASH(IPV6_UDP, 2TUPLE_DST));
+		INSERT_FLAGS(EFX_RX_HASH(IPV6_UDP, 2TUPLE_SRC));
 	}
 
-	LIST_FLAGS(entryp, IPV4, B_FALSE, additional_modes);
-	LIST_FLAGS(entryp, IPV6, B_FALSE, additional_modes);
+	INSERT_FLAGS(EFX_RX_HASH(IPV4_TCP, 2TUPLE));
+	INSERT_FLAGS(EFX_RX_HASH(IPV6_TCP, 2TUPLE));
 
-#undef LIST_FLAGS
+	INSERT_FLAGS(EFX_RX_HASH(IPV4, 2TUPLE));
+	INSERT_FLAGS(EFX_RX_HASH(IPV6, 2TUPLE));
 
-	*nflagsp = (unsigned int)(entryp - flagsp);
-	EFSYS_ASSERT3U(*nflagsp, <=, EFX_RX_HASH_NFLAGS);
+	if (encp->enc_rx_scale_additional_modes_supported != B_FALSE) {
+		INSERT_FLAGS(EFX_RX_HASH(IPV4_TCP, 1TUPLE_DST));
+		INSERT_FLAGS(EFX_RX_HASH(IPV4_TCP, 1TUPLE_SRC));
 
+		INSERT_FLAGS(EFX_RX_HASH(IPV6_TCP, 1TUPLE_DST));
+		INSERT_FLAGS(EFX_RX_HASH(IPV6_TCP, 1TUPLE_SRC));
+
+		INSERT_FLAGS(EFX_RX_HASH(IPV4_UDP, 2TUPLE));
+		INSERT_FLAGS(EFX_RX_HASH(IPV4_UDP, 1TUPLE_DST));
+		INSERT_FLAGS(EFX_RX_HASH(IPV4_UDP, 1TUPLE_SRC));
+
+		INSERT_FLAGS(EFX_RX_HASH(IPV6_UDP, 2TUPLE));
+		INSERT_FLAGS(EFX_RX_HASH(IPV6_UDP, 1TUPLE_DST));
+		INSERT_FLAGS(EFX_RX_HASH(IPV6_UDP, 1TUPLE_SRC));
+
+		INSERT_FLAGS(EFX_RX_HASH(IPV4, 1TUPLE_DST));
+		INSERT_FLAGS(EFX_RX_HASH(IPV4, 1TUPLE_SRC));
+
+		INSERT_FLAGS(EFX_RX_HASH(IPV6, 1TUPLE_DST));
+		INSERT_FLAGS(EFX_RX_HASH(IPV6, 1TUPLE_SRC));
+	}
+
+	INSERT_FLAGS(EFX_RX_HASH(IPV4_TCP, DISABLE));
+	INSERT_FLAGS(EFX_RX_HASH(IPV6_TCP, DISABLE));
+
+	INSERT_FLAGS(EFX_RX_HASH(IPV4_UDP, DISABLE));
+	INSERT_FLAGS(EFX_RX_HASH(IPV6_UDP, DISABLE));
+
+	INSERT_FLAGS(EFX_RX_HASH(IPV4, DISABLE));
+	INSERT_FLAGS(EFX_RX_HASH(IPV6, DISABLE));
+
+#undef INSERT_FLAGS
+
+done:
+	*nflagsp = nflags;
 	return (0);
 
+fail2:
+	EFSYS_PROBE(fail2);
 fail1:
 	EFSYS_PROBE1(fail1, efx_rc_t, rc);
 
@@ -577,7 +610,8 @@ efx_rx_scale_mode_set(
 	/*
 	 * Get the list of supported hash flags and sanitise the input.
 	 */
-	rc = efx_rx_scale_hash_flags_get(enp, alg, type_flags, &type_nflags);
+	rc = efx_rx_scale_hash_flags_get(enp, alg, type_flags,
+				    EFX_ARRAY_SIZE(type_flags), &type_nflags);
 	if (rc != 0)
 		goto fail2;

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