[PATCH ethtool] ethtool: Avoid undefined shifts in bitmap helpers

[PATCH ethtool] ethtool: Avoid undefined shifts in bitmap helpers

[Riten Gupta]

Several helpers that test or set a bit in an array of 32-bit words build
the mask as (1 << (n % 32)). The literal 1 is a signed int, so for every
bit position congruent to 31 modulo 32 the shift is undefined behavior:
2^31 is not representable in a 32-bit int. The unsigned type of the
bitmap word does not change the type of the shift expression.

All of these are reached in normal use:

 - ethtool_link_mode_set_bit() is called by init_global_link_mode_masks()
   on startup, so with -fsanitize=undefined -fno-sanitize-recover=undefined
   every command, including "ethtool --version", aborts with

     internal.h:202:22: runtime error: left shift of 1 by 31 places
     cannot be represented in type 'int'

 - feature_on() and set_sf_req_mask() handle the netdev feature bitmaps
   for "ethtool -k" and "ethtool -K"
 - walk_bitset() handles compact bitsets, e.g. the link mode bitsets
   requested by "ethtool -a"

Use 1U, as bitset_get_bit() and the netlink/monitor.c filter helpers
already do.

Fixes: 33133abf3b77 ("ethtool.c: add support for ETHTOOL_xLINKSETTINGS ioctls")
Fixes: 490503bdbd67 ("netlink: add bitset helpers")
Fixes: f2c17e107900 ("netlink: add netlink handler for gfeatures (-k)")
Fixes: a7a05af4a1ea ("netlink: add netlink handler for sfeatures (-K)")
Suggested-by: Joseph Beshay <jbeshay@meta.com>
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Riten Gupta <ritengupta@meta.com>
---

Notes:
    Tested on top of bae2dbd with gcc 11.5, -fsanitize=undefined
    -fno-sanitize-recover=undefined:
    
     - before: every command aborts in ethtool_link_mode_set_bit() on startup
     - after: --version, -k lo, --json -k lo, and -K/-k round trips on a veth
       pair complete without UBSan reports, including -K tx-esp-segmentation
       and -K hsr-dup-offload (feature bits 31 and 63 here), which go through
       set_sf_req_mask(); walk_bitset() was tested with a synthetic 64-bit
       compact bitset with bits 31 and 63 set
     - all test-cmdline cases pass before and after
    
    test-cmdline reports one more instance of this, RXH_DISCARD (1 << 31) in
    uapi/linux/ethtool.h, used by "-N ... rx-flow-hash ... r". As that header
    is imported from the kernel, I have not touched it here.
    
    The internal.h and features.c changes were previously posted by Jesse
    Brandeburg as 08/13 of a larger cleanup series [1], which stalled on the
    UAPI changes needed for BIT(). This patch is only the minimal fix,
    extended to the same pattern in walk_bitset().
    
    [1] https://lore.kernel.org/netdev/20221208011122.2343363-9-jesse.brandeburg@intel.com/

 internal.h         | 4 ++--
 netlink/bitset.c   | 4 ++--
 netlink/features.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/internal.h b/internal.h
index 50d43cc..406776a 100644
--- a/internal.h
+++ b/internal.h
@@ -192,14 +192,14 @@ static inline int ethtool_link_mode_test_bit(unsigned int nr, const u32 *mask)
 {
 	if (nr >= ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBITS)
 		return !!0;
-	return !!(mask[nr / 32] & (1 << (nr % 32)));
+	return !!(mask[nr / 32] & (1U << (nr % 32)));
 }
 
 static inline int ethtool_link_mode_set_bit(unsigned int nr, u32 *mask)
 {
 	if (nr >= ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBITS)
 		return -1;
-	mask[nr / 32] |= (1 << (nr % 32));
+	mask[nr / 32] |= (1U << (nr % 32));
 	return 0;
 }
 
diff --git a/netlink/bitset.c b/netlink/bitset.c
index 10ce8e9..0c58431 100644
--- a/netlink/bitset.c
+++ b/netlink/bitset.c
@@ -227,9 +227,9 @@ int walk_bitset(const struct nlattr *bitset, const struct stringset *labels,
 		val_bm = mnl_attr_get_payload(bits);
 		mask_bm = mask ? mnl_attr_get_payload(mask) : NULL;
 		for (idx = 0; idx < count; idx++)
-			if (!mask_bm || (mask_bm[idx / 32] & (1 << (idx % 32))))
+			if (!mask_bm || (mask_bm[idx / 32] & (1U << (idx % 32))))
 				cb(idx, get_string(labels, idx),
-				   val_bm[idx / 32] & (1 << (idx % 32)), data);
+				   val_bm[idx / 32] & (1U << (idx % 32)), data);
 		return 0;
 	}
 
diff --git a/netlink/features.c b/netlink/features.c
index 1a8c2f5..2973b44 100644
--- a/netlink/features.c
+++ b/netlink/features.c
@@ -57,7 +57,7 @@ static int prepare_feature_results(const struct nlattr *const *tb,
 
 static bool feature_on(const uint32_t *bitmap, unsigned int idx)
 {
-	return bitmap[idx / 32] & (1 << (idx % 32));
+	return bitmap[idx / 32] & (1U << (idx % 32));
 }
 
 static void dump_feature(const struct feature_results *results,
@@ -304,7 +304,7 @@ static void set_sf_req_mask(struct nl_context *nlctx, unsigned int idx)
 {
 	struct sfeatures_context *sfctx = nlctx->cmd_private;
 
-	sfctx->req_mask[idx / 32] |= (1 << (idx % 32));
+	sfctx->req_mask[idx / 32] |= (1U << (idx % 32));
 }
 
 static int fill_legacy_flag(struct nl_context *nlctx, const char *flag_name,