netdev - Re: [PATCH net 6/8] bonding: disallow setting nested bonding + ipsec offload

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 for Android: free password hash cracker in your pocket

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

Date:   Sat, 3 Jul 2021 15:46:33 +0900
From:   Taehee Yoo <ap420073@...il.com>
To:     Jay Vosburgh <jay.vosburgh@...onical.com>
Cc:     davem@...emloft.net, kuba@...nel.org, vfalico@...il.com,
        andy@...yhouse.net, jesse.brandeburg@...el.com,
        anthony.l.nguyen@...el.com, jarod@...hat.com,
        netdev@...r.kernel.org, intel-wired-lan@...ts.osuosl.org
Subject: Re: [PATCH net 6/8] bonding: disallow setting nested bonding + ipsec
 offload

On 7/3/21 6:26 AM, Jay Vosburgh wrote:
 > Taehee Yoo <ap420073@...il.com> wrote:
 >
 > [...]
 >> @@ -479,8 +481,9 @@ static bool bond_ipsec_offload_ok(struct sk_buff 
*skb, struct xfrm_state *xs)
 >> 	if (BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP)
 >> 		return true;
 >
 > 	Not a question about this patch, but isn't the "return true"
 > above incorrect (i.e., should return false)?  I understand that the
 > ipsec offload is only available for active-backup mode, but the test
 > above will return true for all modes other than active-backup.
 >

I really agree with you.
I tried to test it but I couldn't because my NIC isn't working TX side 
ipsec offload(ixgbevf).
(dev->ndo_dev_offload_okf() is called in only tx side.)
So, I didn't include that change.

Thanks a lot,
Taehee

 > 	-J
 >
 >> -	if (!(slave_dev->xfrmdev_ops
 >> -	      && slave_dev->xfrmdev_ops->xdo_dev_offload_ok)) {
 >> +	if (!slave_dev->xfrmdev_ops ||
 >> +	    !slave_dev->xfrmdev_ops->xdo_dev_offload_ok ||
 >> +	    netif_is_bond_master(slave_dev)) {
 >> 		slave_warn(bond_dev, slave_dev, "%s: no slave 
xdo_dev_offload_ok\n", __func__);
 >> 		return false;
 >> 	}
 >> --
 >> 2.17.1
 >>
 >
 > ---
 > 	-Jay Vosburgh, jay.vosburgh@...onical.com
 >