netdev - Re: [PATCH net-next 4/6] bonding: don't validate arp if we don't have to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <51C22E58.5080209@redhat.com>
Date:	Thu, 20 Jun 2013 00:19:04 +0200
From:	Nikolay Aleksandrov <nikolay@...hat.com>
To:	Veaceslav Falico <vfalico@...hat.com>
CC:	netdev@...r.kernel.org, fubar@...ibm.com, andy@...yhouse.net,
	davem@...emloft.net, linux@...2.net, nicolas.2p.debian@...e.fr,
	rick.jones2@...com
Subject: Re: [PATCH net-next 4/6] bonding: don't validate arp if we don't
 have to

On 19/06/13 19:34, Veaceslav Falico wrote:
> Currently, we validate all the incoming arps if arp_validate not 0.
> However, we don't have to validate backup slaves if arp_validate == active
> and vice versa, so return early in bond_arp_rcv() in these cases.
> 
> It works correctly now because we verify arp_validate in slave_last_rx(),
> however we're just doing useless work in bond_arp_rcv().
> 
> Signed-off-by: Veaceslav Falico <vfalico@...hat.com>
> ---
>  drivers/net/bonding/bond_main.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index b69c7f0..2cfbb2e 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2624,6 +2624,10 @@ static int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
>  		return RX_HANDLER_ANOTHER;
>  
>  	read_lock(&bond->lock);
> +
> +	if (!slave_do_arp_validate(bond, slave))
> +		goto out_unlock;
> +
>  	alen = arp_hdr_len(bond->dev);
>  
>  	pr_debug("bond_arp_rcv: bond %s skb->dev %s\n",
Hm, I think this issue runs deeper because recv_probe can be wrong and
also if arp_validate is enabled while the bond is running then
recv_probe is not set (or unset for that matter if disabled). I have a
patch which needs little more work for some time now in my queue that
fixes this, but if you'd like to fix it I'd suggest addressing that
issue (recv_probe), because then you can just drop these checks and
improve performance when disabled (after it's been enabled).
This got a bit confusing when I read it :-)

Cheers,
 Nik
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html