github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-05-25 15:41:52 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

Merge branch 'net-shaper-fix-valid-confusion-even-more'

Browse Source 
Jakub Kicinski says:

====================
net: shaper: fix VALID confusion even more

Sashiko reported another pre-exising issue in the previous
batch of fixes:
https://sashiko.dev/#/patchset/20260510192904.3987113-7-kuba@kernel.org

Turns out I over-esitmated the guarantees of the XArray flags.
Stop using them completely.
====================

Link: https://patch.msgid.link/20260515221325.1685455-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski 2026-05-20 16:34:21 -07:00
commit b1a736f8bc
2 changed files with 57 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/net/net_shaper.h
View File

@ -53,6 +53,7 @@ struct net_shaper {
/* private: */
u32 leaves; /* accounted only for NODE scope */
bool valid;
struct rcu_head rcu;
};

98
net/shaper/shaper.c
View File

@ -138,35 +138,58 @@ static int net_shaper_fill_handle(struct sk_buff *msg,
return -EMSGSIZE;
}
static void net_shaper_copy(struct net_shaper *dst,
const struct net_shaper *src)
{
WRITE_ONCE(dst->parent.scope, READ_ONCE(src->parent.scope));
WRITE_ONCE(dst->parent.id, READ_ONCE(src->parent.id));
WRITE_ONCE(dst->handle.scope, READ_ONCE(src->handle.scope));
WRITE_ONCE(dst->handle.id, READ_ONCE(src->handle.id));
WRITE_ONCE(dst->metric, READ_ONCE(src->metric));
WRITE_ONCE(dst->bw_min, READ_ONCE(src->bw_min));
WRITE_ONCE(dst->bw_max, READ_ONCE(src->bw_max));
WRITE_ONCE(dst->burst, READ_ONCE(src->burst));
WRITE_ONCE(dst->priority, READ_ONCE(src->priority));
WRITE_ONCE(dst->weight, READ_ONCE(src->weight));
/* private fields are only used on the write path under the lock */
data_race(dst->leaves = src->leaves);
}
static int
net_shaper_fill_one(struct sk_buff *msg,
const struct net_shaper_binding *binding,
const struct net_shaper *shaper,
const struct genl_info *info)
{
struct net_shaper cur;
void *hdr;
hdr = genlmsg_iput(msg, info);
if (!hdr)
return -EMSGSIZE;
/* Make a copy to avoid data races */
net_shaper_copy(&cur, shaper);
if (net_shaper_fill_binding(msg, binding, NET_SHAPER_A_IFINDEX) ||
net_shaper_fill_handle(msg, &shaper->parent,
net_shaper_fill_handle(msg, &cur.parent,
NET_SHAPER_A_PARENT) ||
net_shaper_fill_handle(msg, &shaper->handle,
net_shaper_fill_handle(msg, &cur.handle,
NET_SHAPER_A_HANDLE) ||
((shaper->bw_min || shaper->bw_max || shaper->burst) &&
nla_put_u32(msg, NET_SHAPER_A_METRIC, shaper->metric)) ||
(shaper->bw_min &&
nla_put_uint(msg, NET_SHAPER_A_BW_MIN, shaper->bw_min)) ||
(shaper->bw_max &&
nla_put_uint(msg, NET_SHAPER_A_BW_MAX, shaper->bw_max)) ||
(shaper->burst &&
nla_put_uint(msg, NET_SHAPER_A_BURST, shaper->burst)) ||
(shaper->priority &&
nla_put_u32(msg, NET_SHAPER_A_PRIORITY, shaper->priority)) ||
(shaper->weight &&
nla_put_u32(msg, NET_SHAPER_A_WEIGHT, shaper->weight)))
((cur.bw_min || cur.bw_max || cur.burst) &&
nla_put_u32(msg, NET_SHAPER_A_METRIC, cur.metric)) ||
(cur.bw_min &&
nla_put_uint(msg, NET_SHAPER_A_BW_MIN, cur.bw_min)) ||
(cur.bw_max &&
nla_put_uint(msg, NET_SHAPER_A_BW_MAX, cur.bw_max)) ||
(cur.burst &&
nla_put_uint(msg, NET_SHAPER_A_BURST, cur.burst)) ||
(cur.priority &&
nla_put_u32(msg, NET_SHAPER_A_PRIORITY, cur.priority)) ||
(cur.weight &&
nla_put_u32(msg, NET_SHAPER_A_WEIGHT, cur.weight)))
goto nla_put_failure;
genlmsg_end(msg, hdr);
@ -283,31 +306,24 @@ static void net_shaper_default_parent(const struct net_shaper_handle *handle,
parent->id = 0;
}
/* MARK_0 is already in use due to XA_FLAGS_ALLOC. The VALID mark is set on
* an entry only after the device-side configuration has completed
* successfully (see net_shaper_commit()). Lookups and dumps must filter on
* this mark to avoid exposing tentative entries inserted by
* net_shaper_pre_insert() while the driver call is still in flight.
*/
#define NET_SHAPER_VALID XA_MARK_1
static struct net_shaper *
net_shaper_lookup(struct net_shaper_binding *binding,
const struct net_shaper_handle *handle)
{
u32 index = net_shaper_handle_to_index(handle);
struct net_shaper_hierarchy *hierarchy;
struct net_shaper *cur;
hierarchy = net_shaper_hierarchy_rcu(binding);
if (!hierarchy || !xa_get_mark(&hierarchy->shapers, index,
NET_SHAPER_VALID))
if (!hierarchy)
return NULL;
/* Pairs with smp_wmb() in net_shaper_commit(): if the entry is
* valid, its contents must be visible too.
*/
smp_rmb();
return xa_load(&hierarchy->shapers, index);
cur = xa_load(&hierarchy->shapers, index);
/* Check valid before reading fields */
if (!cur || !smp_load_acquire(&cur->valid))
return NULL;
return cur;
}
/* Allocate on demand the per device shaper's hierarchy container.
@ -421,12 +437,10 @@ static void net_shaper_commit(struct net_shaper_binding *binding,
if (WARN_ON_ONCE(!cur))
continue;
/* Successful update: drop the tentative mark
* and update the hierarchy container.
*/
*cur = shapers[i];
smp_wmb();
__xa_set_mark(&hierarchy->shapers, index, NET_SHAPER_VALID);
/* Successful update: update the hierarchy container... */
net_shaper_copy(cur, &shapers[i]);
/* ... publish to lockless readers. */
smp_store_release(&cur->valid, true);
}
xa_unlock(&hierarchy->shapers);
}
@ -443,10 +457,10 @@ static void net_shaper_rollback(struct net_shaper_binding *binding)
xa_lock(&hierarchy->shapers);
xa_for_each(&hierarchy->shapers, index, cur) {
if (xa_get_mark(&hierarchy->shapers, index, NET_SHAPER_VALID))
if (cur->valid)
continue;
__xa_erase(&hierarchy->shapers, index);
kfree(cur);
kfree_rcu(cur, rcu);
}
xa_unlock(&hierarchy->shapers);
}
@ -859,12 +873,12 @@ int net_shaper_nl_get_dumpit(struct sk_buff *skb,
goto out_unlock;
for (; (shaper = xa_find(&hierarchy->shapers, &ctx->start_index,
U32_MAX, NET_SHAPER_VALID));
U32_MAX, XA_PRESENT));
ctx->start_index++) {
/* Pairs with smp_wmb() in net_shaper_commit(): the entry
* is marked VALID, so its contents must be visible too.
*/
smp_rmb();
/* Check valid before reading fields */
if (!smp_load_acquire(&shaper->valid))
continue;
ret = net_shaper_fill_one(skb, binding, shaper, info);
if (ret)
break;