On Mon, Dec 27, 2021 at 07:21:16PM +0200, Nikolay Aleksandrov wrote: > As reported[1] if startup query interval is set too low in combination with > large number of startup queries and we have multiple bridges or even a > single bridge with multiple querier vlans configured we can crash the > machine. Add a 1 second minimum which must be enforced by overwriting the > value if set lower (i.e. without returning an error) to avoid breaking > user-space. If that happens a log message is emitted to let the admin know > that the startup interval has been set to the minimum. It doesn't make > sense to make the startup interval lower than the normal query interval > so use the same value of 1 second. The issue has been present since these > intervals could be user-controlled. > > [1] https://lore.kernel.org/netdev/e8b9ce41-57b9-b6e2-a46a-ff9c791cf0ba@xxxxxxxxx/ > > Fixes: d902eee43f19 ("bridge: Add multicast count/interval sysfs entries") > Reported-by: Eric Dumazet <eric.dumazet@xxxxxxxxx> > Signed-off-by: Nikolay Aleksandrov <nikolay@xxxxxxxxxx> > --- > net/bridge/br_multicast.c | 16 ++++++++++++++++ > net/bridge/br_netlink.c | 2 +- > net/bridge/br_private.h | 3 +++ > net/bridge/br_sysfs_br.c | 2 +- > net/bridge/br_vlan_options.c | 2 +- > 5 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > index 998da4a2d209..de2409889489 100644 > --- a/net/bridge/br_multicast.c > +++ b/net/bridge/br_multicast.c > @@ -4538,6 +4538,22 @@ void br_multicast_set_query_intvl(struct net_bridge_mcast *brmctx, > brmctx->multicast_query_interval = intvl_jiffies; > } > > +void br_multicast_set_startup_query_intvl(struct net_bridge_mcast *brmctx, > + unsigned long val) > +{ > + unsigned long intvl_jiffies = clock_t_to_jiffies(val); > + > + if (intvl_jiffies < BR_MULTICAST_STARTUP_QUERY_INTVL_MIN) { > + br_info(brmctx->br, > + "trying to set multicast startup query interval below minimum, setting to %lu (%ums)\n", > + jiffies_to_clock_t(BR_MULTICAST_STARTUP_QUERY_INTVL_MIN), > + jiffies_to_msecs(BR_MULTICAST_STARTUP_QUERY_INTVL_MIN)); > + intvl_jiffies = BR_MULTICAST_STARTUP_QUERY_INTVL_MIN; > + } > + > + brmctx->multicast_startup_query_interval = intvl_jiffies; > +} > + > /** > * br_multicast_list_adjacent - Returns snooped multicast addresses > * @dev: The bridge port adjacent to which to retrieve addresses > diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c > index 701dd8b8455e..2ff83d84230d 100644 > --- a/net/bridge/br_netlink.c > +++ b/net/bridge/br_netlink.c > @@ -1369,7 +1369,7 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[], > if (data[IFLA_BR_MCAST_STARTUP_QUERY_INTVL]) { > u64 val = nla_get_u64(data[IFLA_BR_MCAST_STARTUP_QUERY_INTVL]); > > - br->multicast_ctx.multicast_startup_query_interval = clock_t_to_jiffies(val); > + br_multicast_set_startup_query_intvl(&br->multicast_ctx, val); > } > > if (data[IFLA_BR_MCAST_STATS_ENABLED]) { > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index 4ed7f11042e8..2187a0c3fd22 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -29,6 +29,7 @@ > > #define BR_MULTICAST_DEFAULT_HASH_MAX 4096 > #define BR_MULTICAST_QUERY_INTVL_MIN msecs_to_jiffies(1000) > +#define BR_MULTICAST_STARTUP_QUERY_INTVL_MIN BR_MULTICAST_QUERY_INTVL_MIN > > #define BR_HWDOM_MAX BITS_PER_LONG > > @@ -966,6 +967,8 @@ size_t br_multicast_querier_state_size(void); > size_t br_rports_size(const struct net_bridge_mcast *brmctx); > void br_multicast_set_query_intvl(struct net_bridge_mcast *brmctx, > unsigned long val); > +void br_multicast_set_startup_query_intvl(struct net_bridge_mcast *brmctx, > + unsigned long val); > > static inline bool br_group_is_l2(const struct br_ip *group) > { > diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c > index f5bd1114a434..7b0c19772111 100644 > --- a/net/bridge/br_sysfs_br.c > +++ b/net/bridge/br_sysfs_br.c > @@ -706,7 +706,7 @@ static ssize_t multicast_startup_query_interval_show( > static int set_startup_query_interval(struct net_bridge *br, unsigned long val, > struct netlink_ext_ack *extack) > { > - br->multicast_ctx.multicast_startup_query_interval = clock_t_to_jiffies(val); > + br_multicast_set_startup_query_intvl(&br->multicast_ctx, val); > return 0; > } > > diff --git a/net/bridge/br_vlan_options.c b/net/bridge/br_vlan_options.c > index bf1ac0874279..a6382973b3e7 100644 > --- a/net/bridge/br_vlan_options.c > +++ b/net/bridge/br_vlan_options.c > @@ -535,7 +535,7 @@ static int br_vlan_process_global_one_opts(const struct net_bridge *br, > u64 val; > > val = nla_get_u64(tb[BRIDGE_VLANDB_GOPTS_MCAST_STARTUP_QUERY_INTVL]); > - v->br_mcast_ctx.multicast_startup_query_interval = clock_t_to_jiffies(val); > + br_multicast_set_startup_query_intvl(&v->br_mcast_ctx, val); > *changed = true; > } > if (tb[BRIDGE_VLANDB_GOPTS_MCAST_QUERIER]) { > -- > 2.33.1 > <formletter> This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. </formletter>
