ethtool: Fix wrong mod state in case of verbose and 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
6699170376 ("ethtool: fix application of verbose no_mask bitset") 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 value will always
differ to the new value.

Fix it by adding a new function to compare bitmaps and a temporary variable
which save the state of the old bitmap.

Fixes: 6699170376 ("ethtool: fix application of verbose no_mask bitset")
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
Link: https://patch.msgid.link/20241202153358.1142095-1-kory.maincent@bootlin.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Kory Maincent 2024-12-02 16:33:57 +01:00 committed by Jakub Kicinski
parent 50b9420444
commit 910c4788d6

View File

@ -425,12 +425,32 @@ static int ethnl_parse_bit(unsigned int *index, bool *val, unsigned int nbits,
return 0; return 0;
} }
/**
* ethnl_bitmap32_equal() - Compare two bitmaps
* @map1: first bitmap
* @map2: second bitmap
* @nbits: bit size to compare
*
* Return: true if first @nbits are equal, false if not
*/
static bool ethnl_bitmap32_equal(const u32 *map1, const u32 *map2,
unsigned int nbits)
{
if (memcmp(map1, map2, nbits / 32 * sizeof(u32)))
return false;
if (nbits % 32 == 0)
return true;
return !((map1[nbits / 32] ^ map2[nbits / 32]) &
ethnl_lower_bits(nbits % 32));
}
static int static int
ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits, ethnl_update_bitset32_verbose(u32 *bitmap, unsigned int nbits,
const struct nlattr *attr, struct nlattr **tb, const struct nlattr *attr, struct nlattr **tb,
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 *saved_bitmap = NULL;
struct nlattr *bit_attr; struct nlattr *bit_attr;
bool no_mask; bool no_mask;
int rem; int rem;
@ -448,8 +468,20 @@ 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);
bool dummy;
/* 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);
}
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,22 +490,30 @@ 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");
kfree(saved_bitmap);
return -EINVAL; return -EINVAL;
} }
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) {
kfree(saved_bitmap);
return ret; return ret;
}
old_val = bitmap[idx / 32] & ((u32)1 << (idx % 32)); old_val = 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));
else else
bitmap[idx / 32] &= ~((u32)1 << (idx % 32)); bitmap[idx / 32] &= ~((u32)1 << (idx % 32));
if (!no_mask)
*mod = true; *mod = true;
} }
} }
if (no_mask && !ethnl_bitmap32_equal(saved_bitmap, bitmap, nbits))
*mod = true;
kfree(saved_bitmap);
return 0; return 0;
} }