Skip to content

Commit 0f9ea71

Browse files
kuba-mooPaolo Abeni
authored andcommitted
net: shaper: protect late read accesses to the hierarchy
We look up a netdev during prep of Netlink ops (pre- callbacks) and take a ref to it. Then later in the body of the callback we take its lock or RCU which are the actual protections. This is not proper, a conversion from a ref to a locked netdev must include a liveness check (a check if the netdev hasn't been unregistered already). Fix the read cases (those under RCU). Writes needs a separate change to protect from creating the hierarchy after flush has already run. Fixes: 4b623f9 ("net-shapers: implement NL get operation") Reported-by: Paul Moses <[email protected]> Link: https://lore.kernel.org/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Paolo Abeni <[email protected]>
1 parent 8a63baa commit 0f9ea71

1 file changed

Lines changed: 22 additions & 4 deletions

File tree

net/shaper/shaper.c

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,21 @@ net_shaper_hierarchy(struct net_shaper_binding *binding)
6565
return NULL;
6666
}
6767

68+
static struct net_shaper_hierarchy *
69+
net_shaper_hierarchy_rcu(struct net_shaper_binding *binding)
70+
{
71+
/* Readers look up the device and take a ref, then take RCU lock
72+
* later at which point netdev may have been unregistered and flushed.
73+
* READ_ONCE() pairs with WRITE_ONCE() in net_shaper_hierarchy_setup.
74+
*/
75+
if (binding->type == NET_SHAPER_BINDING_TYPE_NETDEV &&
76+
READ_ONCE(binding->netdev->reg_state) <= NETREG_REGISTERED)
77+
return READ_ONCE(binding->netdev->net_shaper_hierarchy);
78+
79+
/* No other type supported yet. */
80+
return NULL;
81+
}
82+
6883
static const struct net_shaper_ops *
6984
net_shaper_ops(struct net_shaper_binding *binding)
7085
{
@@ -251,9 +266,10 @@ static struct net_shaper *
251266
net_shaper_lookup(struct net_shaper_binding *binding,
252267
const struct net_shaper_handle *handle)
253268
{
254-
struct net_shaper_hierarchy *hierarchy = net_shaper_hierarchy(binding);
255269
u32 index = net_shaper_handle_to_index(handle);
270+
struct net_shaper_hierarchy *hierarchy;
256271

272+
hierarchy = net_shaper_hierarchy_rcu(binding);
257273
if (!hierarchy || xa_get_mark(&hierarchy->shapers, index,
258274
NET_SHAPER_NOT_VALID))
259275
return NULL;
@@ -778,17 +794,19 @@ int net_shaper_nl_get_dumpit(struct sk_buff *skb,
778794

779795
/* Don't error out dumps performed before any set operation. */
780796
binding = net_shaper_binding_from_ctx(ctx);
781-
hierarchy = net_shaper_hierarchy(binding);
782-
if (!hierarchy)
783-
return 0;
784797

785798
rcu_read_lock();
799+
hierarchy = net_shaper_hierarchy_rcu(binding);
800+
if (!hierarchy)
801+
goto out_unlock;
802+
786803
for (; (shaper = xa_find(&hierarchy->shapers, &ctx->start_index,
787804
U32_MAX, XA_PRESENT)); ctx->start_index++) {
788805
ret = net_shaper_fill_one(skb, binding, shaper, info);
789806
if (ret)
790807
break;
791808
}
809+
out_unlock:
792810
rcu_read_unlock();
793811

794812
return ret;

0 commit comments

Comments
 (0)