From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756042AbdESTXZ (ORCPT ); Fri, 19 May 2017 15:23:25 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:34251 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753225AbdESTXW (ORCPT ); Fri, 19 May 2017 15:23:22 -0400 Date: Fri, 19 May 2017 15:23:15 -0400 From: Andy Gospodarek To: Jarod Wilson Cc: linux-kernel@vger.kernel.org, Mahesh Bandewar , Jay Vosburgh , Veaceslav Falico , netdev@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH net] bonding: fix randomly populated arp target array Message-ID: <20170519192315.GA15049@C02RW35GFVH8.dhcp.broadcom.net> References: <20170519184646.42572-1-jarod@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170519184646.42572-1-jarod@redhat.com> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 19, 2017 at 02:46:46PM -0400, Jarod Wilson wrote: > In commit dc9c4d0fe023, the arp_target array moved from a static global > to a local variable. By the nature of static globals, the array used to > be initialized to all 0. At present, it's full of random data, which > that gets interpreted as arp_target values, when none have actually been > specified. Systems end up booting with spew along these lines: > > [ 32.161783] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready > [ 32.168475] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready > [ 32.175089] 8021q: adding VLAN 0 to HW filter on device lacp0 > [ 32.193091] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready > [ 32.204892] lacp0: Setting MII monitoring interval to 100 > [ 32.211071] lacp0: Removing ARP target 216.124.228.17 > [ 32.216824] lacp0: Removing ARP target 218.160.255.255 > [ 32.222646] lacp0: Removing ARP target 185.170.136.184 > [ 32.228496] lacp0: invalid ARP target 255.255.255.255 specified for removal > [ 32.236294] lacp0: option arp_ip_target: invalid value (-255.255.255.255) > [ 32.243987] lacp0: Removing ARP target 56.125.228.17 > [ 32.249625] lacp0: Removing ARP target 218.160.255.255 > [ 32.255432] lacp0: Removing ARP target 15.157.233.184 > [ 32.261165] lacp0: invalid ARP target 255.255.255.255 specified for removal > [ 32.268939] lacp0: option arp_ip_target: invalid value (-255.255.255.255) > [ 32.276632] lacp0: Removing ARP target 16.0.0.0 > [ 32.281755] lacp0: Removing ARP target 218.160.255.255 > [ 32.287567] lacp0: Removing ARP target 72.125.228.17 > [ 32.293165] lacp0: Removing ARP target 218.160.255.255 > [ 32.298970] lacp0: Removing ARP target 8.125.228.17 > [ 32.304458] lacp0: Removing ARP target 218.160.255.255 > > None of these were actually specified as ARP targets, and the driver does > seem to clean up the mess okay, but it's rather noisy and confusing, leaks > values to userspace, and the 255.255.255.255 spew shows up even when debug > prints are disabled. > > The fix: just zero out arp_target at init time. > > While we're in here, init arp_all_targets_value in the right place. > Looks good. Thanks, Jarod! Acked-by: Andy Gospodarek > Fixes: dc9c4d0fe023 ("bonding: reduce scope of some global variables") > CC: Mahesh Bandewar > CC: Jay Vosburgh > CC: Veaceslav Falico > CC: Andy Gospodarek > CC: netdev@vger.kernel.org > CC: stable@vger.kernel.org > Signed-off-by: Jarod Wilson > --- > drivers/net/bonding/bond_main.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > index 2be78807fd6e..73313318399c 100644 > --- a/drivers/net/bonding/bond_main.c > +++ b/drivers/net/bonding/bond_main.c > @@ -4271,10 +4271,10 @@ static int bond_check_params(struct bond_params *params) > int arp_validate_value, fail_over_mac_value, primary_reselect_value, i; > struct bond_opt_value newval; > const struct bond_opt_value *valptr; > - int arp_all_targets_value; > + int arp_all_targets_value = 0; > u16 ad_actor_sys_prio = 0; > u16 ad_user_port_key = 0; > - __be32 arp_target[BOND_MAX_ARP_TARGETS]; > + __be32 arp_target[BOND_MAX_ARP_TARGETS] = { 0 }; > int arp_ip_count; > int bond_mode = BOND_MODE_ROUNDROBIN; > int xmit_hashtype = BOND_XMIT_POLICY_LAYER2; > @@ -4501,7 +4501,6 @@ static int bond_check_params(struct bond_params *params) > arp_validate_value = 0; > } > > - arp_all_targets_value = 0; > if (arp_all_targets) { > bond_opt_initstr(&newval, arp_all_targets); > valptr = bond_opt_parse(bond_opt_get(BOND_OPT_ARP_ALL_TARGETS), > -- > 2.12.1 >