* [PATCH net v2] bridge: cfm: reject invalid CCM interval at configuration time
@ 2026-04-05 0:03 Xiang Mei
2026-04-06 14:15 ` Simon Horman
2026-04-07 7:07 ` Ido Schimmel
0 siblings, 2 replies; 4+ messages in thread
From: Xiang Mei @ 2026-04-05 0:03 UTC (permalink / raw)
To: netdev
Cc: horms, bridge, razor, idosch, davem, edumazet, pabeni, bestswngs,
Xiang Mei
ccm_tx_work_expired() re-arms itself via queue_delayed_work() using
the configured exp_interval converted by interval_to_us(). When
exp_interval is BR_CFM_CCM_INTERVAL_NONE or out of range,
interval_to_us() returns 0, causing the worker to fire immediately in
a tight loop that allocates skbs until OOM.
Fix this by validating exp_interval at configuration time:
- Constrain IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL to [1, 7] in the
netlink policy so userspace cannot set an invalid value.
- Reject starting CCM TX in br_cfm_cc_ccm_tx() when exp_interval has
not yet been configured (defaults to 0 from kzalloc).
Fixes: a806ad8ee2aa ("bridge: cfm: Kernel space implementation of CFM. CCM frame TX added.")
Reported-by: Weiming Shi <bestswngs@gmail•com>
Signed-off-by: Xiang Mei <xmei5@asu•edu>
---
v2: Move validation out of the datapath and into configuration
net/bridge/br_cfm.c | 6 ++++++
net/bridge/br_cfm_netlink.c | 2 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c
index 118c7ea48c35..dea56fffa1c1 100644
--- a/net/bridge/br_cfm.c
+++ b/net/bridge/br_cfm.c
@@ -805,6 +805,12 @@ int br_cfm_cc_ccm_tx(struct net_bridge *br, const u32 instance,
goto save;
}
+ if (!interval_to_us(mep->cc_config.exp_interval)) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Invalid CCM interval");
+ return -EINVAL;
+ }
+
/* Start delayed work to transmit CCM frames. It is done with zero delay
* to send first frame immediately
*/
diff --git a/net/bridge/br_cfm_netlink.c b/net/bridge/br_cfm_netlink.c
index 2faab44652e7..1bb33c8f587b 100644
--- a/net/bridge/br_cfm_netlink.c
+++ b/net/bridge/br_cfm_netlink.c
@@ -34,7 +34,7 @@ br_cfm_cc_config_policy[IFLA_BRIDGE_CFM_CC_CONFIG_MAX + 1] = {
[IFLA_BRIDGE_CFM_CC_CONFIG_UNSPEC] = { .type = NLA_REJECT },
[IFLA_BRIDGE_CFM_CC_CONFIG_INSTANCE] = { .type = NLA_U32 },
[IFLA_BRIDGE_CFM_CC_CONFIG_ENABLE] = { .type = NLA_U32 },
- [IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL] = { .type = NLA_U32 },
+ [IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL] = NLA_POLICY_RANGE(NLA_U32, 1, 7),
[IFLA_BRIDGE_CFM_CC_CONFIG_EXP_MAID] = {
.type = NLA_BINARY, .len = CFM_MAID_LENGTH },
};
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net v2] bridge: cfm: reject invalid CCM interval at configuration time
2026-04-05 0:03 [PATCH net v2] bridge: cfm: reject invalid CCM interval at configuration time Xiang Mei
@ 2026-04-06 14:15 ` Simon Horman
2026-04-07 7:07 ` Ido Schimmel
1 sibling, 0 replies; 4+ messages in thread
From: Simon Horman @ 2026-04-06 14:15 UTC (permalink / raw)
To: Xiang Mei
Cc: netdev, bridge, razor, idosch, davem, edumazet, pabeni, bestswngs
On Sat, Apr 04, 2026 at 05:03:24PM -0700, Xiang Mei wrote:
> ccm_tx_work_expired() re-arms itself via queue_delayed_work() using
> the configured exp_interval converted by interval_to_us(). When
> exp_interval is BR_CFM_CCM_INTERVAL_NONE or out of range,
> interval_to_us() returns 0, causing the worker to fire immediately in
> a tight loop that allocates skbs until OOM.
>
> Fix this by validating exp_interval at configuration time:
>
> - Constrain IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL to [1, 7] in the
> netlink policy so userspace cannot set an invalid value.
>
> - Reject starting CCM TX in br_cfm_cc_ccm_tx() when exp_interval has
> not yet been configured (defaults to 0 from kzalloc).
>
> Fixes: a806ad8ee2aa ("bridge: cfm: Kernel space implementation of CFM. CCM frame TX added.")
> Reported-by: Weiming Shi <bestswngs@gmail•com>
> Signed-off-by: Xiang Mei <xmei5@asu•edu>
> ---
> v2: Move validation out of the datapath and into configuration
Thanks for the update.
Reviewed-by: Simon Horman <horms@kernel•org>
...
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net v2] bridge: cfm: reject invalid CCM interval at configuration time
2026-04-05 0:03 [PATCH net v2] bridge: cfm: reject invalid CCM interval at configuration time Xiang Mei
2026-04-06 14:15 ` Simon Horman
@ 2026-04-07 7:07 ` Ido Schimmel
2026-06-06 22:00 ` Xiang Mei
1 sibling, 1 reply; 4+ messages in thread
From: Ido Schimmel @ 2026-04-07 7:07 UTC (permalink / raw)
To: Xiang Mei
Cc: netdev, horms, bridge, razor, davem, edumazet, pabeni, bestswngs
On Sat, Apr 04, 2026 at 05:03:24PM -0700, Xiang Mei wrote:
> ccm_tx_work_expired() re-arms itself via queue_delayed_work() using
> the configured exp_interval converted by interval_to_us(). When
> exp_interval is BR_CFM_CCM_INTERVAL_NONE or out of range,
> interval_to_us() returns 0, causing the worker to fire immediately in
> a tight loop that allocates skbs until OOM.
>
> Fix this by validating exp_interval at configuration time:
>
> - Constrain IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL to [1, 7] in the
> netlink policy so userspace cannot set an invalid value.
>
> - Reject starting CCM TX in br_cfm_cc_ccm_tx() when exp_interval has
> not yet been configured (defaults to 0 from kzalloc).
>
> Fixes: a806ad8ee2aa ("bridge: cfm: Kernel space implementation of CFM. CCM frame TX added.")
Nit: Doesn't matter in practice, but let's blame commit 2be665c3940d
("bridge: cfm: Netlink SET configuration Interface.") instead as I don't
think this bug could be triggered before exposing the netlink API.
> Reported-by: Weiming Shi <bestswngs@gmail•com>
> Signed-off-by: Xiang Mei <xmei5@asu•edu>
> ---
> v2: Move validation out of the datapath and into configuration
>
> net/bridge/br_cfm.c | 6 ++++++
> net/bridge/br_cfm_netlink.c | 2 +-
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c
> index 118c7ea48c35..dea56fffa1c1 100644
> --- a/net/bridge/br_cfm.c
> +++ b/net/bridge/br_cfm.c
> @@ -805,6 +805,12 @@ int br_cfm_cc_ccm_tx(struct net_bridge *br, const u32 instance,
> goto save;
> }
>
> + if (!interval_to_us(mep->cc_config.exp_interval)) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "Invalid CCM interval");
> + return -EINVAL;
> + }
> +
> /* Start delayed work to transmit CCM frames. It is done with zero delay
> * to send first frame immediately
> */
> diff --git a/net/bridge/br_cfm_netlink.c b/net/bridge/br_cfm_netlink.c
> index 2faab44652e7..1bb33c8f587b 100644
> --- a/net/bridge/br_cfm_netlink.c
> +++ b/net/bridge/br_cfm_netlink.c
> @@ -34,7 +34,7 @@ br_cfm_cc_config_policy[IFLA_BRIDGE_CFM_CC_CONFIG_MAX + 1] = {
> [IFLA_BRIDGE_CFM_CC_CONFIG_UNSPEC] = { .type = NLA_REJECT },
> [IFLA_BRIDGE_CFM_CC_CONFIG_INSTANCE] = { .type = NLA_U32 },
> [IFLA_BRIDGE_CFM_CC_CONFIG_ENABLE] = { .type = NLA_U32 },
> - [IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL] = NLA_POLICY_RANGE(NLA_U32, 1, 7),
Use BR_CFM_CCM_INTERVAL_3_3_MS and BR_CFM_CCM_INTERVAL_10_MIN instead of
the magic numbers?
The Sashiko review points out that blocking BR_CFM_CCM_INTERVAL_NONE
might break user space, but it seems weird to allow passing a value that
is interpreted the same as an invalid one. Worst case, if someone
complains, we can revert and go back to v1.
> [IFLA_BRIDGE_CFM_CC_CONFIG_EXP_MAID] = {
> .type = NLA_BINARY, .len = CFM_MAID_LENGTH },
> };
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net v2] bridge: cfm: reject invalid CCM interval at configuration time
2026-04-07 7:07 ` Ido Schimmel
@ 2026-06-06 22:00 ` Xiang Mei
0 siblings, 0 replies; 4+ messages in thread
From: Xiang Mei @ 2026-06-06 22:00 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, horms, bridge, razor, davem, edumazet, pabeni, bestswngs
Thanks for the feedback, and sorry for the delayed v3. It was sent.
Best,
Xiang
On Tue, Apr 7, 2026 at 12:07 AM Ido Schimmel <idosch@nvidia•com> wrote:
>
> On Sat, Apr 04, 2026 at 05:03:24PM -0700, Xiang Mei wrote:
> > ccm_tx_work_expired() re-arms itself via queue_delayed_work() using
> > the configured exp_interval converted by interval_to_us(). When
> > exp_interval is BR_CFM_CCM_INTERVAL_NONE or out of range,
> > interval_to_us() returns 0, causing the worker to fire immediately in
> > a tight loop that allocates skbs until OOM.
> >
> > Fix this by validating exp_interval at configuration time:
> >
> > - Constrain IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL to [1, 7] in the
> > netlink policy so userspace cannot set an invalid value.
> >
> > - Reject starting CCM TX in br_cfm_cc_ccm_tx() when exp_interval has
> > not yet been configured (defaults to 0 from kzalloc).
> >
> > Fixes: a806ad8ee2aa ("bridge: cfm: Kernel space implementation of CFM. CCM frame TX added.")
>
> Nit: Doesn't matter in practice, but let's blame commit 2be665c3940d
> ("bridge: cfm: Netlink SET configuration Interface.") instead as I don't
> think this bug could be triggered before exposing the netlink API.
>
> > Reported-by: Weiming Shi <bestswngs@gmail•com>
> > Signed-off-by: Xiang Mei <xmei5@asu•edu>
> > ---
> > v2: Move validation out of the datapath and into configuration
> >
> > net/bridge/br_cfm.c | 6 ++++++
> > net/bridge/br_cfm_netlink.c | 2 +-
> > 2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/bridge/br_cfm.c b/net/bridge/br_cfm.c
> > index 118c7ea48c35..dea56fffa1c1 100644
> > --- a/net/bridge/br_cfm.c
> > +++ b/net/bridge/br_cfm.c
> > @@ -805,6 +805,12 @@ int br_cfm_cc_ccm_tx(struct net_bridge *br, const u32 instance,
> > goto save;
> > }
> >
> > + if (!interval_to_us(mep->cc_config.exp_interval)) {
> > + NL_SET_ERR_MSG_MOD(extack,
> > + "Invalid CCM interval");
> > + return -EINVAL;
> > + }
> > +
> > /* Start delayed work to transmit CCM frames. It is done with zero delay
> > * to send first frame immediately
> > */
> > diff --git a/net/bridge/br_cfm_netlink.c b/net/bridge/br_cfm_netlink.c
> > index 2faab44652e7..1bb33c8f587b 100644
> > --- a/net/bridge/br_cfm_netlink.c
> > +++ b/net/bridge/br_cfm_netlink.c
> > @@ -34,7 +34,7 @@ br_cfm_cc_config_policy[IFLA_BRIDGE_CFM_CC_CONFIG_MAX + 1] = {
> > [IFLA_BRIDGE_CFM_CC_CONFIG_UNSPEC] = { .type = NLA_REJECT },
> > [IFLA_BRIDGE_CFM_CC_CONFIG_INSTANCE] = { .type = NLA_U32 },
> > [IFLA_BRIDGE_CFM_CC_CONFIG_ENABLE] = { .type = NLA_U32 },
> > - [IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL] = { .type = NLA_U32 },
> > + [IFLA_BRIDGE_CFM_CC_CONFIG_EXP_INTERVAL] = NLA_POLICY_RANGE(NLA_U32, 1, 7),
>
> Use BR_CFM_CCM_INTERVAL_3_3_MS and BR_CFM_CCM_INTERVAL_10_MIN instead of
> the magic numbers?
>
> The Sashiko review points out that blocking BR_CFM_CCM_INTERVAL_NONE
> might break user space, but it seems weird to allow passing a value that
> is interpreted the same as an invalid one. Worst case, if someone
> complains, we can revert and go back to v1.
>
> > [IFLA_BRIDGE_CFM_CC_CONFIG_EXP_MAID] = {
> > .type = NLA_BINARY, .len = CFM_MAID_LENGTH },
> > };
> > --
> > 2.43.0
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-06-06 22:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-05 0:03 [PATCH net v2] bridge: cfm: reject invalid CCM interval at configuration time Xiang Mei
2026-04-06 14:15 ` Simon Horman
2026-04-07 7:07 ` Ido Schimmel
2026-06-06 22:00 ` Xiang Mei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox