Commit 108a36d0 authored by Kory Maincent's avatar Kory Maincent Committed by Jakub Kicinski

ethtool: Fix mod state of verbose no_mask bitset

A bitset without mask in a _SET request means we want exactly the bits in
the bitset to be set. This works correctly for compact format but when
verbose format is parsed, ethnl_update_bitset32_verbose() only sets the
bits present in the request bitset but does not clear the rest. The commit
66991703 fixes this issue by clearing the whole target bitmap before we
start iterating. The solution proposed brought an issue with the behavior
of the mod variable. As the bitset is always cleared the old val will
always differ to the new val.

Fix it by adding a new temporary variable which save the state of the old
bitmap.

Fixes: 66991703 ("ethtool: fix application of verbose no_mask bitset")
Signed-off-by: default avatarKory Maincent <kory.maincent@bootlin.com>
Reviewed-by: default avatarSimon Horman <horms@kernel.org>
Link: https://lore.kernel.org/r/20231009133645.44503-1-kory.maincent@bootlin.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent b52acd02
...@@ -431,8 +431,10 @@ ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits, ...@@ -431,8 +431,10 @@ ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits,
ethnl_string_array_t names, ethnl_string_array_t names,
struct netlink_ext_ack *extack, bool *mod) struct netlink_ext_ack *extack, bool *mod)
{ {
u32 *orig_bitmap, *saved_bitmap = NULL;
struct nlattr *bit_attr; struct nlattr *bit_attr;
bool no_mask; bool no_mask;
bool dummy;
int rem; int rem;
int ret; int ret;
...@@ -448,8 +450,22 @@ ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits, ...@@ -448,8 +450,22 @@ ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits,
} }
no_mask = tb[ETHTOOL_A_BITSET_NOMASK]; no_mask = tb[ETHTOOL_A_BITSET_NOMASK];
if (no_mask) if (no_mask) {
ethnl_bitmap32_clear(bitmap, 0, nbits, mod); unsigned int nwords = DIV_ROUND_UP(nbits, 32);
unsigned int nbytes = nwords * sizeof(u32);
/* The bitmap size is only the size of the map part without
* its mask part.
*/
saved_bitmap = kcalloc(nwords, sizeof(u32), GFP_KERNEL);
if (!saved_bitmap)
return -ENOMEM;
memcpy(saved_bitmap, bitmap, nbytes);
ethnl_bitmap32_clear(bitmap, 0, nbits, &dummy);
orig_bitmap = saved_bitmap;
} else {
orig_bitmap = bitmap;
}
nla_for_each_nested(bit_attr, tb[ETHTOOL_A_BITSET_BITS], rem) { nla_for_each_nested(bit_attr, tb[ETHTOOL_A_BITSET_BITS], rem) {
bool old_val, new_val; bool old_val, new_val;
...@@ -458,13 +474,14 @@ ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits, ...@@ -458,13 +474,14 @@ ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits,
if (nla_type(bit_attr) != ETHTOOL_A_BITSET_BITS_BIT) { if (nla_type(bit_attr) != ETHTOOL_A_BITSET_BITS_BIT) {
NL_SET_ERR_MSG_ATTR(extack, bit_attr, NL_SET_ERR_MSG_ATTR(extack, bit_attr,
"only ETHTOOL_A_BITSET_BITS_BIT allowed in ETHTOOL_A_BITSET_BITS"); "only ETHTOOL_A_BITSET_BITS_BIT allowed in ETHTOOL_A_BITSET_BITS");
return -EINVAL; ret = -EINVAL;
goto out;
} }
ret = ethnl_parse_bit(&idx, &new_val, nbits, bit_attr, no_mask, ret = ethnl_parse_bit(&idx, &new_val, nbits, bit_attr, no_mask,
names, extack); names, extack);
if (ret < 0) if (ret < 0)
return ret; goto out;
old_val = bitmap[idx / 32] & ((u32)1 << (idx % 32)); old_val = orig_bitmap[idx / 32] & ((u32)1 << (idx % 32));
if (new_val != old_val) { if (new_val != old_val) {
if (new_val) if (new_val)
bitmap[idx / 32] |= ((u32)1 << (idx % 32)); bitmap[idx / 32] |= ((u32)1 << (idx % 32));
...@@ -474,7 +491,10 @@ ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits, ...@@ -474,7 +491,10 @@ ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits,
} }
} }
return 0; ret = 0;
out:
kfree(saved_bitmap);
return ret;
} }
static int ethnl_compact_sanity_checks(unsigned int nbits, static int ethnl_compact_sanity_checks(unsigned int nbits,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment