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 Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Riten Gupta --- 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,