From patchwork Thu Apr 23 13:39:04 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 26368 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@bilbo.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id AE3C1B7067 for ; Thu, 23 Apr 2009 23:39:30 +1000 (EST) Received: by ozlabs.org (Postfix) id 9F8E0DE0F3; Thu, 23 Apr 2009 23:39:30 +1000 (EST) Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 41B07DE03F for ; Thu, 23 Apr 2009 23:39:30 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755388AbZDWNjZ (ORCPT ); Thu, 23 Apr 2009 09:39:25 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754859AbZDWNjZ (ORCPT ); Thu, 23 Apr 2009 09:39:25 -0400 Received: from gw1.cosmosbay.com ([212.99.114.194]:52379 "EHLO gw1.cosmosbay.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753667AbZDWNjY convert rfc822-to-8bit (ORCPT ); Thu, 23 Apr 2009 09:39:24 -0400 Received: from [127.0.0.1] (localhost [127.0.0.1]) by gw1.cosmosbay.com (8.13.7/8.13.7) with ESMTP id n3NDd5is020152; Thu, 23 Apr 2009 15:39:05 +0200 Message-ID: <49F06F78.4030601@cosmosbay.com> Date: Thu, 23 Apr 2009 15:39:04 +0200 From: Eric Dumazet User-Agent: Thunderbird 2.0.0.21 (Windows/20090302) MIME-Version: 1.0 To: Jiri Pirko CC: "David S. Miller" , Linux Netdev List , Jay Vosburgh Subject: Re: [PATCH] bonding: bond_slave_info_query() fix References: <49F04FD7.7080508@cosmosbay.com> <20090423121121.GG29268@psychotron.englab.brq.redhat.com> In-Reply-To: <20090423121121.GG29268@psychotron.englab.brq.redhat.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-1.6 (gw1.cosmosbay.com [0.0.0.0]); Thu, 23 Apr 2009 15:39:05 +0200 (CEST) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Jiri Pirko a écrit : > Thu, Apr 23, 2009 at 01:24:07PM CEST, dada1@cosmosbay.com wrote: >> bond_slave_info_query() should keep a read lock while accessing slave info, >> or risk accessing stale data and corruption. >> >> Signed-off-by: Eric Dumazet >> >> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >> index 63369b6..66697db 100644 >> --- a/drivers/net/bonding/bond_main.c >> +++ b/drivers/net/bonding/bond_main.c >> @@ -2213,33 +2213,27 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in >> { >> struct bonding *bond = netdev_priv(bond_dev); >> struct slave *slave; >> - int i, found = 0; >> + int i, ret = -ENODEV; >> >> - if (info->slave_id < 0) { >> + if (info->slave_id < 0) >> return -ENODEV; > > You can return ret; here since -ENODEV is there, but I don't know if it isn't > against policy. Patch looks good anyway. > Yes, we can just delete this extra test, since requested slave_id wont be found anyway if negative. Thanks [PATCH] bonding: bond_slave_info_query() fix bond_slave_info_query() should keep a read lock while accessing slave info, or risk accessing stale data and corruption. Signed-off-by: Eric Dumazet Signed-off-by: Jay Vosburgh --- 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 diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 63369b6..67515b7 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2213,33 +2213,24 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in { struct bonding *bond = netdev_priv(bond_dev); struct slave *slave; - int i, found = 0; - - if (info->slave_id < 0) { - return -ENODEV; - } + int i, res = -ENODEV; read_lock(&bond->lock); bond_for_each_slave(bond, slave, i) { if (i == (int)info->slave_id) { - found = 1; + res = 0; + strcpy(info->slave_name, slave->dev->name); + info->link = slave->link; + info->state = slave->state; + info->link_failure_count = slave->link_failure_count; break; } } read_unlock(&bond->lock); - if (found) { - strcpy(info->slave_name, slave->dev->name); - info->link = slave->link; - info->state = slave->state; - info->link_failure_count = slave->link_failure_count; - } else { - return -ENODEV; - } - - return 0; + return res; } /*-------------------------------- Monitoring -------------------------------*/