[PATCH ethtool 0/2] fix RSS hash-function reply handling

[PATCH ethtool 0/2] fix RSS hash-function reply handling

[Prabhakar Pujeri]

The first patch keeps RSS hash-function masks at their full 32-bit width
and bounds shifts in both text and JSON output.  The second removes a
duplicate parse of the same netlink reply.

Tested with make check in the default and --disable-netlink builds.

Prabhakar Pujeri (2):
  rss: bound hash function masks in text and JSON output
  rss: drop duplicate attribute parsing in rss_reply_cb()

 netlink/rss.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

-- 
2.55.0

[PATCH ethtool 1/2] rss: bound hash function masks in text and JSON output

[Prabhakar Pujeri]

The RSS hash-function string set can contain more names than fit in the
32-bit ETHTOOL_A_RSS_HFUNC mask.  Iterating over every name can therefore
shift by 32 bits or more.  The JSON helper also accepts the mask as u8,
which drops its upper 24 bits.

Keep the JSON mask as u32, bound both output loops to the mask width, and
use unsigned shifts so bit 31 is well-defined.

Signed-off-by: Prabhakar Pujeri <prabhakar.pujeri@dell.com>
---
 netlink/rss.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/netlink/rss.c b/netlink/rss.c
index 83cc504..85c2bd9 100644
--- a/netlink/rss.c
+++ b/netlink/rss.c
@@ -21,11 +21,15 @@ struct cb_args {
 
 void dump_json_rss_info(struct cmd_context *ctx, u32 *indir_table,
 			u32 indir_size, u8 *hkey, u32 hkey_size,
-			const struct stringset *hash_funcs, u8 hfunc,
+			const struct stringset *hash_funcs, u32 hfunc,
 			u32 input_xfrm)
 {
+	unsigned int hfunc_count = get_count(hash_funcs);
 	unsigned int i;
 
+	if (hfunc_count > sizeof(hfunc) * BITS_PER_BYTE)
+		hfunc_count = sizeof(hfunc) * BITS_PER_BYTE;
+
 	open_json_object(NULL);
 	print_string(PRINT_JSON, "ifname", NULL, ctx->devname);
 	if (indir_size) {
@@ -43,15 +47,15 @@ void dump_json_rss_info(struct cmd_context *ctx, u32 *indir_table,
 	}
 
 	if (hfunc) {
-		for (i = 0; i < get_count(hash_funcs); i++) {
-			if (hfunc & (1 << i)) {
+		for (i = 0; i < hfunc_count; i++) {
+			if (hfunc & (1U << i)) {
 				print_string(PRINT_JSON, "rss-hash-function",
 					     NULL, get_string(hash_funcs, i));
 				break;
 			}
 		}
 
-		if (i == get_count(hash_funcs))
+		if (i == hfunc_count)
 			print_uint(PRINT_JSON, "rss-hash-function-raw", NULL, hfunc);
 	}
 
@@ -159,6 +163,8 @@ int rss_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 				   hkey, hkey_bytes, hash_funcs, rss_hfunc,
 				   input_xfrm);
 	} else {
+		unsigned int hfunc_count = get_count(hash_funcs);
+
 		print_indir_table(nlctx->ctx, args->num_rings,
 				  indir_size, (u32 *)indir_table);
 		print_rss_hkey(hkey, hkey_bytes);
@@ -167,10 +173,16 @@ int rss_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 			printf("    Operation not supported\n");
 			return 0;
 		}
-		for (unsigned int i = 0; i < get_count(hash_funcs); i++) {
+		/* rss_hfunc is a 32-bit mask; if the kernel reports more
+		 * hash function names than that, cap the loop to avoid
+		 * out-of-range shifts.
+		 */
+		if (hfunc_count > sizeof(rss_hfunc) * BITS_PER_BYTE)
+			hfunc_count = sizeof(rss_hfunc) * BITS_PER_BYTE;
+		for (unsigned int i = 0; i < hfunc_count; i++) {
 			printf("    %s: %s\n", get_string(hash_funcs, i),
-			       (rss_hfunc & (1 << i)) ? "on" : "off");
-			rss_hfunc &= ~(1 << i);
+			       (rss_hfunc & (1U << i)) ? "on" : "off");
+			rss_hfunc &= ~(1U << i);
 		}
 		if (rss_hfunc)
 			printf("    Unknown hash function: 0x%x\n", rss_hfunc);
-- 
2.55.0

[PATCH ethtool 2/2] rss: drop duplicate attribute parsing in rss_reply_cb()

[Prabhakar Pujeri]

The second mnl_attr_parse() call re-parses the very same message into
the same array as the call at the top of the function, and none of the
code in between touches the table or the values extracted from it:
all attribute reads happen before it and attr_cb() just rewrites the
table entries with identical pointers.

Remove the redundant call; its failure semantics were identical to the
first parse anyway (MNL_CB_OK when silent, MNL_CB_ERROR otherwise).

Signed-off-by: Prabhakar Pujeri <prabhakar.pujeri@dell.com>
---
 netlink/rss.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/netlink/rss.c b/netlink/rss.c
index 85c2bd9..8c70c29 100644
--- a/netlink/rss.c
+++ b/netlink/rss.c
@@ -149,10 +149,6 @@ int rss_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 	hash_funcs = global_stringset(ETH_SS_RSS_HASH_FUNCS,
 				      nlctx->ethnl2_socket);
 
-	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
-	if (ret < 0)
-		return silent ? MNL_CB_OK : MNL_CB_ERROR;
-
 	ret = get_num_rings(args);
 	if (ret < 0)
 		return MNL_CB_ERROR;
-- 
2.55.0