From: Riten Gupta <ritengupta@meta.com>
To: Michal Kubecek <mkubecek@suse.cz>
Cc: <netdev@vger.kernel.org>, Joseph Beshay <jbeshay@meta.com>,
"Jesse Brandeburg" <jbrandeb@kernel.org>
Subject: [PATCH ethtool] ethtool: Avoid undefined shifts in bitmap helpers
Date: Mon, 9 Nov 2026 15:21:44 -0800 [thread overview]
Message-ID: <20261109232251.1105094-1-ritengupta@meta.com> (raw)
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,
reply other threads:[~2026-11-09 23:23 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261109232251.1105094-1-ritengupta@meta.com \
--to=ritengupta@meta.com \
--cc=jbeshay@meta.com \
--cc=jbrandeb@kernel.org \
--cc=mkubecek@suse.cz \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox