netdev - Re: [PATCH] bonding: avoid re-entry of bond

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]

Date:	Fri, 19 Dec 2014 10:11:57 -0500
From:	Andy Gospodarek <gospo@...ulusnetworks.com>
To:	Wengang Wang <wen.gang.wang@...cle.com>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] bonding: avoid re-entry of bond_release

On Fri, Dec 19, 2014 at 04:56:57PM +0800, Wengang Wang wrote:
> If bond_release is run against an interface which is already detached from
> it's master, then there is an error message shown like
> 	"<master name> cannot release <slave name>".
> 
> The call path is:
> 	bond_do_ioctl()
> 		bond_release()
> 			__bond_release_one()
> 
> Though it does not really harm, the message the message is misleading.
> This patch tries to avoid the message.
> 
> Signed-off-by: Wengang Wang <wen.gang.wang@...cle.com>
> ---
>  drivers/net/bonding/bond_main.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 184c434..4a71bbd 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3256,7 +3256,10 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
>  		break;
>  	case BOND_RELEASE_OLD:
>  	case SIOCBONDRELEASE:
> -		res = bond_release(bond_dev, slave_dev);
> +		if (slave_dev->flags & IFF_SLAVE)
> +			res = bond_release(bond_dev, slave_dev);
> +		else
> +			res = 0;

Functionally this patch is fine, but I would prefer that you simply
change the check in __bond_release_one to not be so noisy.  There is a
check[1] in bond_enslave to see if a slave is already in a bond and that
just prints a message of netdev_dbg (rather than netdev_err) and it
seems that would be appropriate for this type of message.

[1] from bond_enslave():

        /* already enslaved */
        if (slave_dev->flags & IFF_SLAVE) {
                netdev_dbg(bond_dev, "Error: Device was already enslaved\n");
                return -EBUSY;
        }


>  		break;
>  	case BOND_SETHWADDR_OLD:
>  	case SIOCBONDSETHWADDR:
> -- 
> 1.8.3.1
> 
> --
> 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
--
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