thr3ads.net - Linux Ethernet Bridging - [Bridge] [PATCH net 1/2] net: bridge: mcast: add and enforce query interval minimum [Jan 2022]

If this information is useful, please help other people find it:
Share via:

Nikolay Aleksandrov

2021-Dec-27 17:21 UTC

[Bridge] [PATCH net 1/2] net: bridge: mcast: add and enforce query interval minimum

As reported[1] if query interval is set too low 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 administrator know that the interval has been set to the minimum.
The issue has been present since these intervals could be user-controlled.

[1] https://lore.kernel.org/netdev/e8b9ce41-57b9-b6e2-a46a-ff9c791cf0ba at
gmail.com/

Fixes: d902eee43f19 ("bridge: Add multicast count/interval sysfs
entries")
Reported-by: Eric Dumazet <eric.dumazet at gmail.com>
Signed-off-by: Nikolay Aleksandrov <nikolay at nvidia.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 f3d751105343..998da4a2d209 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -4522,6 +4522,22 @@ int br_multicast_set_mld_version(struct net_bridge_mcast
*brmctx,
 }
 #endif
 
+void br_multicast_set_query_intvl(struct net_bridge_mcast *brmctx,
+				  unsigned long val)
+{
+	unsigned long intvl_jiffies = clock_t_to_jiffies(val);
+
+	if (intvl_jiffies < BR_MULTICAST_QUERY_INTVL_MIN) {
+		br_info(brmctx->br,
+			"trying to set multicast query interval below minimum, setting to %lu
(%ums)\n",
+			jiffies_to_clock_t(BR_MULTICAST_QUERY_INTVL_MIN),
+			jiffies_to_msecs(BR_MULTICAST_QUERY_INTVL_MIN));
+		intvl_jiffies = BR_MULTICAST_QUERY_INTVL_MIN;
+	}
+
+	brmctx->multicast_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 0c8b5f1a15bc..701dd8b8455e 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1357,7 +1357,7 @@ static int br_changelink(struct net_device *brdev, struct
nlattr *tb[],
 	if (data[IFLA_BR_MCAST_QUERY_INTVL]) {
 		u64 val = nla_get_u64(data[IFLA_BR_MCAST_QUERY_INTVL]);
 
-		br->multicast_ctx.multicast_query_interval = clock_t_to_jiffies(val);
+		br_multicast_set_query_intvl(&br->multicast_ctx, val);
 	}
 
 	if (data[IFLA_BR_MCAST_QUERY_RESPONSE_INTVL]) {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index c0efd697865a..4ed7f11042e8 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -28,6 +28,7 @@
 #define BR_MAX_PORTS	(1<<BR_PORT_BITS)
 
 #define BR_MULTICAST_DEFAULT_HASH_MAX 4096
+#define BR_MULTICAST_QUERY_INTVL_MIN msecs_to_jiffies(1000)
 
 #define BR_HWDOM_MAX BITS_PER_LONG
 
@@ -963,6 +964,8 @@ int br_multicast_dump_querier_state(struct sk_buff *skb,
 				    int nest_attr);
 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);
 
 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 d9a89ddd0331..f5bd1114a434 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -658,7 +658,7 @@ static ssize_t multicast_query_interval_show(struct device
*d,
 static int set_query_interval(struct net_bridge *br, unsigned long val,
 			      struct netlink_ext_ack *extack)
 {
-	br->multicast_ctx.multicast_query_interval = clock_t_to_jiffies(val);
+	br_multicast_set_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 8ffd4ed2563c..bf1ac0874279 100644
--- a/net/bridge/br_vlan_options.c
+++ b/net/bridge/br_vlan_options.c
@@ -521,7 +521,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_QUERY_INTVL]);
-		v->br_mcast_ctx.multicast_query_interval = clock_t_to_jiffies(val);
+		br_multicast_set_query_intvl(&v->br_mcast_ctx, val);
 		*changed = true;
 	}
 	if (tb[BRIDGE_VLANDB_GOPTS_MCAST_QUERY_RESPONSE_INTVL]) {
-- 
2.33.1

Greg KH

2021-Dec-28 07:48 UTC

head link

[Bridge] [PATCH net 1/2] net: bridge: mcast: add and enforce query interval minimum

On Mon, Dec 27, 2021 at 07:21:15PM +0200, Nikolay Aleksandrov
wrote:> As reported[1] if query interval is set too low 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 administrator know that the interval has been set to the minimum.
> The issue has been present since these intervals could be user-controlled.
> 
> [1] https://lore.kernel.org/netdev/e8b9ce41-57b9-b6e2-a46a-ff9c791cf0ba at
gmail.com/
> 
> Fixes: d902eee43f19 ("bridge: Add multicast count/interval sysfs
entries")
> Reported-by: Eric Dumazet <eric.dumazet at gmail.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay at nvidia.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 f3d751105343..998da4a2d209 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -4522,6 +4522,22 @@ int br_multicast_set_mld_version(struct
net_bridge_mcast *brmctx,
>  }
>  #endif
>  
> +void br_multicast_set_query_intvl(struct net_bridge_mcast *brmctx,
> +				  unsigned long val)
> +{
> +	unsigned long intvl_jiffies = clock_t_to_jiffies(val);
> +
> +	if (intvl_jiffies < BR_MULTICAST_QUERY_INTVL_MIN) {
> +		br_info(brmctx->br,
> +			"trying to set multicast query interval below minimum, setting to
%lu (%ums)\n",
> +			jiffies_to_clock_t(BR_MULTICAST_QUERY_INTVL_MIN),
> +			jiffies_to_msecs(BR_MULTICAST_QUERY_INTVL_MIN));
> +		intvl_jiffies = BR_MULTICAST_QUERY_INTVL_MIN;
> +	}
> +
> +	brmctx->multicast_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 0c8b5f1a15bc..701dd8b8455e 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -1357,7 +1357,7 @@ static int br_changelink(struct net_device *brdev,
struct nlattr *tb[],
>  	if (data[IFLA_BR_MCAST_QUERY_INTVL]) {
>  		u64 val = nla_get_u64(data[IFLA_BR_MCAST_QUERY_INTVL]);
>  
> -		br->multicast_ctx.multicast_query_interval = clock_t_to_jiffies(val);
> +		br_multicast_set_query_intvl(&br->multicast_ctx, val);
>  	}
>  
>  	if (data[IFLA_BR_MCAST_QUERY_RESPONSE_INTVL]) {
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index c0efd697865a..4ed7f11042e8 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -28,6 +28,7 @@
>  #define BR_MAX_PORTS	(1<<BR_PORT_BITS)
>  
>  #define BR_MULTICAST_DEFAULT_HASH_MAX 4096
> +#define BR_MULTICAST_QUERY_INTVL_MIN msecs_to_jiffies(1000)
>  
>  #define BR_HWDOM_MAX BITS_PER_LONG
>  
> @@ -963,6 +964,8 @@ int br_multicast_dump_querier_state(struct sk_buff
*skb,
>  				    int nest_attr);
>  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);
>  
>  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 d9a89ddd0331..f5bd1114a434 100644
> --- a/net/bridge/br_sysfs_br.c
> +++ b/net/bridge/br_sysfs_br.c
> @@ -658,7 +658,7 @@ static ssize_t multicast_query_interval_show(struct
device *d,
>  static int set_query_interval(struct net_bridge *br, unsigned long val,
>  			      struct netlink_ext_ack *extack)
>  {
> -	br->multicast_ctx.multicast_query_interval = clock_t_to_jiffies(val);
> +	br_multicast_set_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 8ffd4ed2563c..bf1ac0874279 100644
> --- a/net/bridge/br_vlan_options.c
> +++ b/net/bridge/br_vlan_options.c
> @@ -521,7 +521,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_QUERY_INTVL]);
> -		v->br_mcast_ctx.multicast_query_interval = clock_t_to_jiffies(val);
> +		br_multicast_set_query_intvl(&v->br_mcast_ctx, val);
>  		*changed = true;
>  	}
>  	if (tb[BRIDGE_VLANDB_GOPTS_MCAST_QUERY_RESPONSE_INTVL]) {
> -- 
> 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>

Eric Dumazet

2022-Jan-05 18:14 UTC

head link

[Bridge] [PATCH net 1/2] net: bridge: mcast: add and enforce query interval minimum

On 12/27/21 09:21, Nikolay Aleksandrov wrote:> As reported[1] if query interval is set too low 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 administrator know that the interval has been set to the minimum.
> The issue has been present since these intervals could be user-controlled.
>
> [1] https://lore.kernel.org/netdev/e8b9ce41-57b9-b6e2-a46a-ff9c791cf0ba at
gmail.com/
>
> Fixes: d902eee43f19 ("bridge: Add multicast count/interval sysfs
entries")
> Reported-by: Eric Dumazet <eric.dumazet at gmail.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay at nvidia.com>
> ---

Reviewed-by: Eric Dumazet <edumazet at google.com>

Thanks !

Linux Ethernet Bridging - Jan 2022 - [Bridge] [PATCH net 1/2] net: bridge: mcast: add and enforce query interval minimum

[Bridge] [PATCH net 1/2] net: bridge: mcast: add and enforce query interval minimum

[Bridge] [PATCH net 1/2] net: bridge: mcast: add and enforce query interval minimum

[Bridge] [PATCH net 1/2] net: bridge: mcast: add and enforce query interval minimum