[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YcrBVOqvlscLiJNi@kroah.com>
Date: Tue, 28 Dec 2021 08:48:36 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Nikolay Aleksandrov <nikolay@...dia.com>
Cc: netdev@...r.kernel.org, eric.dumazet@...il.com,
stable@...r.kernel.org, herbert@...dor.apana.org.au,
roopa@...dia.com, davem@...emloft.net,
bridge@...ts.linux-foundation.org, kuba@...nel.org
Subject: Re: [PATCH net 2/2] net: bridge: mcast: add and enforce startup
query interval minimum
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@gmail.com/
>
> Fixes: d902eee43f19 ("bridge: Add multicast count/interval sysfs entries")
> Reported-by: Eric Dumazet <eric.dumazet@...il.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay@...dia.com>
> ---
> 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>
Powered by blists - more mailing lists