net_shaper_commit() overrides nodes which may be concurrently read
under RCU. This is not a huge deal since the entries only contain
config, worst case user will see inconsistent config params. But
we should try to avoid this obvious RCU violation. Try to allocate
a new node. Since commit() can't fail fall back to overriding.
Full fix is probably not worth the complexity, struct net_shaper
is around 80B, and the allocation is with GFP_KERNEL.
Fixes: 93954b40f6a4 ("net-shapers: implement NL set and delete operations")
Signed-off-by: Jakub Kicinski <[email protected]>
---
net/shaper/shaper.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c
index 692e38df42cb..a2e9adca9afc 100644
--- a/net/shaper/shaper.c
+++ b/net/shaper/shaper.c
@@ -406,18 +406,36 @@ static void net_shaper_commit(struct net_shaper_binding
*binding,
int i;
for (i = 0; i < nr_shapers; ++i) {
+ struct net_shaper *new;
+
index = net_shaper_handle_to_index(&shapers[i].handle);
cur = xa_load(&hierarchy->shapers, index);
if (WARN_ON_ONCE(!cur))
continue;
- /* Successful update: drop the tentative mark
- * and update the hierarchy container.
+ /* If the shaper is already visible try to allocate a new
+ * entry so that we don't cause torn reads under RCU.
+ * This tiny GFP_KERNEL alloc should never fail, really,
+ * but if it does just override, the torn read is acceptable.
*/
- *cur = shapers[i];
- smp_wmb();
- xa_set_mark(&hierarchy->shapers, index, NET_SHAPER_VALID);
+ new = NULL;
+ if (xa_get_mark(&hierarchy->shapers, index, NET_SHAPER_VALID))
+ new = kmalloc_obj(*new);
+
+ if (new) {
+ *new = shapers[i];
+ smp_wmb();
+ /* Replacing an entry, xa_store() can't fail */
+ WARN_ON(xa_err(xa_store(&hierarchy->shapers, index,
+ new, GFP_KERNEL)));
+ kfree_rcu(cur, rcu);
+ } else {
+ *cur = shapers[i];
+ smp_wmb();
+ xa_set_mark(&hierarchy->shapers, index,
+ NET_SHAPER_VALID);
+ }
}
}
--
2.54.0
