netdev - Re: [PATCH net 2/2] IB/ipoib: Fix netlink support in IPoIB

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: <20171231202308.GA10145@ziepe.ca>
Date:   Sun, 31 Dec 2017 13:23:08 -0700
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Denis Drozdov <denisd@...lanox.com>
Cc:     davem@...emloft.net, dledford@...hat.com, leonro@...lanox.com,
        linux-rdma@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH net 2/2] IB/ipoib: Fix netlink support in IPoIB

On Sun, Dec 31, 2017 at 01:16:23PM +0200, Denis Drozdov wrote:
> IPoIB netlink support was broken by commit cd565b4b51e5
> ("IB/IPoIB: Support acceleration options callbacks"),
> that added flow which allocates netdev rdma structures
> after netlink object is already created. Such situation leads
> to crash in __ipoib_device_add, once trying to reuse netlink
> device.
> This commit restores the netlink support.
> 
> Fixes: cd565b4b51e5 ("IB/IPoIB: Support acceleration options callbacks")
> Reviewed-by: Erez Shitrit <erezsh@...lanox.com>
> Reviewed-by: Leon Romanovsky <leonro@...lanox.com>
> Reviewed-by: Saeed Mahameed <saeedm@...lanox.com>
> Signed-off-by: Denis Drozdov <denisd@...lanox.com>
>  drivers/infiniband/ulp/ipoib/ipoib.h         |  2 ++
>  drivers/infiniband/ulp/ipoib/ipoib_main.c    | 23 +++++++++--------
>  drivers/infiniband/ulp/ipoib/ipoib_netlink.c | 38 +++++++++++++++++++++++++---
>  drivers/infiniband/ulp/ipoib/ipoib_vlan.c    | 20 ++++-----------
>  4 files changed, 54 insertions(+), 29 deletions(-)

I assume this is going through DaveM's tree? In which case for the
drivers/infiniband part:

Acked-by: Jason Gunthorpe <jgg@...lanox.com>

> @@ -131,11 +157,15 @@ static int ipoib_new_child_link(struct net *src_net, struct net_device *dev,
>  	 */
>  	child_pkey |= 0x8000;
>  
> -	err = __ipoib_vlan_add(ppriv, ipoib_priv(dev),
> -			       child_pkey, IPOIB_RTNL_CHILD);
> +	down_write(&ppriv->vlan_rwsem);
> +
> +	priv = ipoib_priv(dev);
> +	err = __ipoib_vlan_add(ppriv, priv, child_pkey, IPOIB_RTNL_CHILD);
> +	up_write(&ppriv->vlan_rwsem);

This looks like a necessary and correct change, but does it need to be
a seperate bug fix commit?

Jason