Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752429AbbD2Tpc (ORCPT ); Wed, 29 Apr 2015 15:45:32 -0400 Received: from mail-wi0-f182.google.com ([209.85.212.182]:36272 "EHLO mail-wi0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752199AbbD2TpY (ORCPT ); Wed, 29 Apr 2015 15:45:24 -0400 Message-ID: <554134D1.7070509@blackwall.org> Date: Wed, 29 Apr 2015 21:45:21 +0200 From: Nikolay Aleksandrov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Pai , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Fix Kernel Panic in bonding driver debugfs file: rlb_hash_table References: In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2303 Lines: 71 On 04/29/2015 08:24 PM, Pai wrote: > This patch fixes a Kernel Panic in bonding driver debugfs file: rlb_hash_table. > > $> modprobe bonding mode=6 > $> cat /sys/kernel/debug/bonding/bond0/rlb_hash_table > > This will crash the kernel. The struct alb_bond_info is initialized only when > the bonding interface is initialized (ip link set bond0 up) and not at the time > it is allocated. If we try to read the table before that, it'll result in a > kernel panic. > > The patch applies against both net and net-next > > Signed-off-by: Vishwanath Pai > Good catch, a few cosmetic nits below. > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > index 089a402..806892a 100644 > --- a/drivers/net/bonding/bond_main.c > +++ b/drivers/net/bonding/bond_main.c > @@ -4510,6 +4510,8 @@ unsigned int bond_get_num_tx_queues(void) > int bond_create(struct net *net, const char *name) > { > struct net_device *bond_dev; > + struct bonding *bond; > + struct alb_bond_info *bond_info; Please arrange these longest to shortest line, it's easier to read. > int res; > > rtnl_lock(); > @@ -4523,6 +4525,14 @@ int bond_create(struct net *net, const char *name) > return -ENOMEM; > } > > + /* > + * Initialize rx_hashtbl_used_head to RLB_NULL_INDEX. > + * It is set to 0 by default which is wrong. > + */ Multiline comment style of networking content is: /* text * text */ See: Documentation/networking/netdev-FAQ.txt Alternatively you can create an inline with descriptive name that does the initialization and wouldn't need the comment. Either way's fine by me. Cheers, Nik > + bond = netdev_priv(bond_dev); > + bond_info = &(BOND_ALB_INFO(bond)); > + bond_info->rx_hashtbl_used_head = RLB_NULL_INDEX; > + > dev_net_set(bond_dev, net); > bond_dev->rtnl_link_ops = &bond_link_ops; > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/