The usual way of inserting entries which are not yet fully ready
into XArray is to have a VALID flag. The shaper code has a NOT_VALID
flag. Since XArray code does not let us create entries with a marks
already set - the creation of entries is currently not atomic.
Flip the polarity of the VALID flag. This closes the tiny race
in net_shaper_pre_insert() of entries being created without
the NOT_VALID flag.
Fixes: 93954b40f6a4 ("net-shapers: implement NL set and delete operations")
Signed-off-by: Jakub Kicinski <[email protected]>
---
net/shaper/shaper.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c
index e28d20774713..c5cf10543af0 100644
--- a/net/shaper/shaper.c
+++ b/net/shaper/shaper.c
@@ -275,11 +275,13 @@ 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, can't reuse such flag as
- * it's cleared by xa_store().
+/* 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_NOT_VALID XA_MARK_1
+#define NET_SHAPER_VALID XA_MARK_1
static struct net_shaper *
net_shaper_lookup(struct net_shaper_binding *binding,
@@ -289,8 +291,8 @@ net_shaper_lookup(struct net_shaper_binding *binding,
struct net_shaper_hierarchy *hierarchy;
hierarchy = net_shaper_hierarchy_rcu(binding);
- if (!hierarchy || xa_get_mark(&hierarchy->shapers, index,
- NET_SHAPER_NOT_VALID))
+ if (!hierarchy || !xa_get_mark(&hierarchy->shapers, index,
+ NET_SHAPER_VALID))
return NULL;
return xa_load(&hierarchy->shapers, index);
@@ -370,11 +372,10 @@ static int net_shaper_pre_insert(struct
net_shaper_binding *binding,
goto free_id;
}
- /* Mark 'tentative' shaper inside the hierarchy container.
- * xa_set_mark is a no-op if the previous store fails.
+ /* Insert as 'tentative' (no VALID mark). The mark will be set by
+ * net_shaper_commit() once the driver-side configuration succeeds.
*/
prev = xa_store(&hierarchy->shapers, index, cur, GFP_KERNEL);
- xa_set_mark(&hierarchy->shapers, index, NET_SHAPER_NOT_VALID);
if (xa_err(prev)) {
NL_SET_ERR_MSG(extack, "Can't insert shaper into device store");
kfree_rcu(cur, rcu);
@@ -410,8 +411,7 @@ static void net_shaper_commit(struct net_shaper_binding
*binding,
/* Successful update: drop the tentative mark
* and update the hierarchy container.
*/
- xa_clear_mark(&hierarchy->shapers, index,
- NET_SHAPER_NOT_VALID);
+ xa_set_mark(&hierarchy->shapers, index, NET_SHAPER_VALID);
*cur = shapers[i];
}
}
@@ -426,8 +426,9 @@ static void net_shaper_rollback(struct net_shaper_binding
*binding)
if (!hierarchy)
return;
- xa_for_each_marked(&hierarchy->shapers, index, cur,
- NET_SHAPER_NOT_VALID) {
+ xa_for_each(&hierarchy->shapers, index, cur) {
+ if (xa_get_mark(&hierarchy->shapers, index, NET_SHAPER_VALID))
+ continue;
xa_erase(&hierarchy->shapers, index);
kfree(cur);
}
@@ -830,7 +831,8 @@ int net_shaper_nl_get_dumpit(struct sk_buff *skb,
goto out_unlock;
for (; (shaper = xa_find(&hierarchy->shapers, &ctx->start_index,
- U32_MAX, XA_PRESENT)); ctx->start_index++) {
+ U32_MAX, NET_SHAPER_VALID));
+ ctx->start_index++) {
ret = net_shaper_fill_one(skb, binding, shaper, info);
if (ret)
break;
--
2.54.0
