Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-16 Thread Erez Shitrit
On Wed, Apr 15, 2015 at 7:06 PM, Jason Gunthorpe
 wrote:
> On Wed, Apr 15, 2015 at 09:17:14AM +0300, Erez Shitrit wrote:
>> >>+   /* parent interface */
>> >>+   if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags))
>> >>+   return dev->ifindex;
>> >>+
>> >>+   /* child/vlan interface */
>> >>+   if (!priv->parent)
>> >>+   return -1;
>
>> >Like was said for other drivers, I can't see how parent can be null
>> >while IPOIB_FLAG_SUBINTERFACE is set. Drop the last if.
>
>> It can, at least for ipoib child interface (AKA "vlan"), you can't
>> control the call for that ndo and it can be called before the parent
>> was set.
>
> If the ndo can be called before the netdev private structures are fully
> prepared then we have another bug, and returning -1 or 0 is not the right
> answer anyhow.
>
> For safety, fold this into your patch.

OK, will do that.

>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c 
> b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
> index 9fad7b5ac8b9..e62b007adf5d 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
> @@ -58,6 +58,7 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct 
> ipoib_dev_priv *priv,
> /* MTU will be reset when mcast join happens */
> priv->dev->mtu   = IPOIB_UD_MTU(priv->max_ib_mtu);
> priv->mcast_mtu  = priv->admin_mtu = priv->dev->mtu;
> +   priv->parent = ppriv->dev;
> set_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags);
>
> result = ipoib_set_dev_features(priv, ppriv->ca);
> @@ -84,8 +85,6 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct 
> ipoib_dev_priv *priv,
> goto register_failed;
> }
>
> -   priv->parent = ppriv->dev;
> -
> ipoib_create_debug_files(priv->dev);
>
> /* RTNL childs don't need proprietary sysfs entries */
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-15 Thread Jason Gunthorpe
On Wed, Apr 15, 2015 at 09:17:14AM +0300, Erez Shitrit wrote:
> >>+   /* parent interface */
> >>+   if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags))
> >>+   return dev->ifindex;
> >>+
> >>+   /* child/vlan interface */
> >>+   if (!priv->parent)
> >>+   return -1;

> >Like was said for other drivers, I can't see how parent can be null
> >while IPOIB_FLAG_SUBINTERFACE is set. Drop the last if.

> It can, at least for ipoib child interface (AKA "vlan"), you can't
> control the call for that ndo and it can be called before the parent
> was set.

If the ndo can be called before the netdev private structures are fully
prepared then we have another bug, and returning -1 or 0 is not the right
answer anyhow.

For safety, fold this into your patch.

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c 
b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
index 9fad7b5ac8b9..e62b007adf5d 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
@@ -58,6 +58,7 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct 
ipoib_dev_priv *priv,
/* MTU will be reset when mcast join happens */
priv->dev->mtu   = IPOIB_UD_MTU(priv->max_ib_mtu);
priv->mcast_mtu  = priv->admin_mtu = priv->dev->mtu;
+   priv->parent = ppriv->dev;
set_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags);
 
result = ipoib_set_dev_features(priv, ppriv->ca);
@@ -84,8 +85,6 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct 
ipoib_dev_priv *priv,
goto register_failed;
}
 
-   priv->parent = ppriv->dev;
-
ipoib_create_debug_files(priv->dev);
 
/* RTNL childs don't need proprietary sysfs entries */
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-15 Thread Honggang LI
On Tue, Apr 14, 2015 at 07:30:03PM +0300, Erez Shitrit wrote:
> > @@ -846,7 +846,10 @@ static int ipoib_get_iflink(const struct net_device 
> > *dev)
> >  {
> > struct ipoib_dev_priv *priv = netdev_priv(dev);
> >
> > -   return priv->parent->ifindex;
> > +   if (priv && priv->parent)
> > +   return priv->parent->ifindex;
> > +   else
> > +   return 0;
> This will make parent interface to return 0 instead of its own ifindex.
> I would suggest write something like that:
> 
> +   /* parent interface */
> +   if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags))
> +   return dev->ifindex;
> +

Hi, Erez

Sorry for delay of reply. It was about 01:00 in the morning, so I
went into bed. And thank you for the suggestion. You are right. After 
insert some printk statements in the driver, I confirmed it.

-- console log 
ipoib_get_iflink: priv = 880275e487c0, priv->parent = (null), priv->flags = 
0x20f, dev = 880275e48000, dev->name = qib_ib1
qib_ib1, test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags) = 0
qib_ib1, idev->ifindex = 14

ipoib_get_iflink: priv = 8802765d27c0, priv->parent = (null), priv->flags = 
0x20f, dev = 8802765d2000, dev->name = qib_ib2
qib_ib2, test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags) = 0
qib_ib2, idev->ifindex = 15

ipoib_get_iflink: priv = 8804741a47c0, priv->parent = 880275e48000, 
priv->flags = 0x224, dev = 8804741a4000, dev->name = qib_ib1.8003
qib_ib1.8003, test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags) = 1
qib_ib1.8003, idev->ifindex = 16

-- console log 

I will rewrite the patch.

> +   /* child/vlan interface */
> +   if (!priv->parent)
> +   return -1;
> +
> return priv->parent->ifindex;
> 
> Thanks,
> Erez.
> 
> >  }
> >
> >  static u32 ipoib_addr_hash(struct ipoib_neigh_hash *htbl, u8 *daddr)
> > --
> > 1.8.3.1
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Honggang LI

There was network accident in the office. I can't find this email in
public mailing list. So, I reset it. If you had recived this, please
ignore it. 

thanks

On Wed, Apr 15, 2015 at 01:16:40PM +0800, Honggang LI wrote:
> On Tue, Apr 14, 2015 at 07:30:03PM +0300, Erez Shitrit wrote:
> > > @@ -846,7 +846,10 @@ static int ipoib_get_iflink(const struct net_device 
> > > *dev)
> > >  {
> > > struct ipoib_dev_priv *priv = netdev_priv(dev);
> > >
> > > -   return priv->parent->ifindex;
> > > +   if (priv && priv->parent)
> > > +   return priv->parent->ifindex;
> > > +   else
> > > +   return 0;
> > This will make parent interface to return 0 instead of its own ifindex.
> > I would suggest write something like that:
> > 
> > +   /* parent interface */
> > +   if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags))
> > +   return dev->ifindex;
> > +
> 
> Hi, Erez
> 
> Sorry for delay of reply. It was about 01:00 in the morning, so I
> went into bed. And thank you for the suggestion. You are right. After 
> insert some printk statements in the driver, I confirmed it.
> 
> -- console log 
> ipoib_get_iflink: priv = 880275e487c0, priv->parent = (null), priv->flags 
> = 0x20f, dev = 880275e48000, dev->name = qib_ib1
> qib_ib1, test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags) = 0
> qib_ib1, idev->ifindex = 14
> 
> ipoib_get_iflink: priv = 8802765d27c0, priv->parent = (null), priv->flags 
> = 0x20f, dev = 8802765d2000, dev->name = qib_ib2
> qib_ib2, test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags) = 0
> qib_ib2, idev->ifindex = 15
> 
> ipoib_get_iflink: priv = 8804741a47c0, priv->parent = 880275e48000, 
> priv->flags = 0x224, dev = 8804741a4000, dev->name = qib_ib1.8003
> qib_ib1.8003, test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags) = 1
> qib_ib1.8003, idev->ifindex = 16
> 
> -- console log 
> 
> I will rewrite the patch.
> 
> > +   /* child/vlan interface */
> > +   if (!priv->parent)
> > +   return -1;
> > +
> > return priv->parent->ifindex;
> > 
> > Thanks,
> > Erez.
> > 
> > >  }
> > >
> > >  static u32 ipoib_addr_hash(struct ipoib_neigh_hash *htbl, u8 *daddr)
> > > --
> > > 1.8.3.1
> > >
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Erez Shitrit

On 4/14/2015 11:41 PM, Jason Gunthorpe wrote:

On Tue, Apr 14, 2015 at 07:30:03PM +0300, Erez Shitrit wrote:


diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 657b89b..11ea6e2 100644
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -846,7 +846,10 @@ static int ipoib_get_iflink(const struct net_device *dev)
  {
 struct ipoib_dev_priv *priv = netdev_priv(dev);

-   return priv->parent->ifindex;
+   if (priv && priv->parent)
+   return priv->parent->ifindex;
+   else
+   return 0;

This will make parent interface to return 0 instead of its own ifindex.
I would suggest write something like that:

Agree


+   /* parent interface */
+   if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags))
+   return dev->ifindex;
+
+   /* child/vlan interface */
+   if (!priv->parent)
+   return -1;

Like was said for other drivers, I can't see how parent can be null
while IPOIB_FLAG_SUBINTERFACE is set. Drop the last if.
It can, at least for ipoib child interface (AKA "vlan"), you can't 
control the call for that ndo and it can be called before the parent was 
set.

Erez, you basically rewrote this, please make a proper patch with the
Fixes and Reported-By credit for Honggang. Lets merge this through
Dave M's tree right away.

Thank you all

Jason
.



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Or Gerlitz

On 4/14/2015 11:41 PM, Jason Gunthorpe wrote:


Erez, you basically rewrote this, please make a proper patch with the Fixes and 
Reported-By credit for Honggang. Lets merge this through Dave M's tree right 
away.


Agree, Erez, add proper Fixes: XXX note and send a patch to netdev 
against net-next. No need for the lengthy crash dump there. Nicolas, 
next time you patch IPoIB, please cc linux-rdma.

Or.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Jason Gunthorpe
On Tue, Apr 14, 2015 at 07:30:03PM +0300, Erez Shitrit wrote:

> > diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
> > b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > index 657b89b..11ea6e2 100644
> > +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > @@ -846,7 +846,10 @@ static int ipoib_get_iflink(const struct net_device 
> > *dev)
> >  {
> > struct ipoib_dev_priv *priv = netdev_priv(dev);
> >
> > -   return priv->parent->ifindex;
> > +   if (priv && priv->parent)
> > +   return priv->parent->ifindex;
> > +   else
> > +   return 0;
> This will make parent interface to return 0 instead of its own ifindex.
> I would suggest write something like that:

Agree

> +   /* parent interface */
> +   if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags))
> +   return dev->ifindex;
> +
> +   /* child/vlan interface */
> +   if (!priv->parent)
> +   return -1;

Like was said for other drivers, I can't see how parent can be null
while IPOIB_FLAG_SUBINTERFACE is set. Drop the last if.

Erez, you basically rewrote this, please make a proper patch with the
Fixes and Reported-By credit for Honggang. Lets merge this through
Dave M's tree right away.

Thank you all

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Nicolas Dichtel

Le 14/04/2015 18:30, Erez Shitrit a écrit :

On Tue, Apr 14, 2015 at 6:20 PM, Honggang Li  wrote:

[snip]

This will make parent interface to return 0 instead of its own ifindex.
I would suggest write something like that:

+   /* parent interface */
+   if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags))
+   return dev->ifindex;
+
+   /* child/vlan interface */
+   if (!priv->parent)
+   return -1;

'return 0' here.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Nicolas Dichtel

Le 14/04/2015 18:01, Yann Droneaud a écrit :
[snip]

Here is the tag:
Fixes: 5aa7add8f14b ("infiniband/ipoib: implement ndo_get_iflink")



Pardon me, but this patch was never submitted to
linux-rdma@vger.kernel.org for review !?

Sorry for that, I missed it. Only Roland Dreier was CCed.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Erez Shitrit
On Tue, Apr 14, 2015 at 6:20 PM, Honggang Li  wrote:
>

[...]

Hi,

> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
> b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index 657b89b..11ea6e2 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -846,7 +846,10 @@ static int ipoib_get_iflink(const struct net_device *dev)
>  {
> struct ipoib_dev_priv *priv = netdev_priv(dev);
>
> -   return priv->parent->ifindex;
> +   if (priv && priv->parent)
> +   return priv->parent->ifindex;
> +   else
> +   return 0;
This will make parent interface to return 0 instead of its own ifindex.
I would suggest write something like that:

+   /* parent interface */
+   if (!test_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags))
+   return dev->ifindex;
+
+   /* child/vlan interface */
+   if (!priv->parent)
+   return -1;
+
return priv->parent->ifindex;

Thanks,
Erez.

>  }
>
>  static u32 ipoib_addr_hash(struct ipoib_neigh_hash *htbl, u8 *daddr)
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Eric Dumazet
On Tue, 2015-04-14 at 23:53 +0800, Honggang LI wrote:
> On Tue, Apr 14, 2015 at 05:49:55PM +0200, Nicolas Dichtel wrote:
> > Le 14/04/2015 17:44, Honggang LI a écrit :
> > >On Tue, Apr 14, 2015 at 08:34:33AM -0700, Eric Dumazet wrote:
> > >>On Tue, 2015-04-14 at 23:20 +0800, Honggang Li wrote:
> > >>>Starting monitoring for VG vg_rdma01:   3 logical volume(s) in volume
> > >>>group "vg_rdma01" monitored
> > >>>[  OK  ]
> > >>
> > >>
> > >>>CR2: 0120
> > >>>---[ end trace a8610f6e9640eb85 ]---
> > >>>
> > >>>Signed-off-by: Honggang Li 
> > >>
> > >>When was this bug added ?
> > >>
> > >
> > >Sorry, I do not know. I ran into this bug today when I'm tracing a race
> > >condition issue related qib. In order to check upstream linux-next fix
> > >the race condition or not, I build linux-next-20150414 on two machines. 
> > >Both
> > >machines panic at modprobe ib_ipoib. Do you means I need to report a
> > >bug? But I do not know report to who or where.
> > 
> > Here is the tag:
> > Fixes: 5aa7add8f14b ("infiniband/ipoib: implement ndo_get_iflink")
> >
> 
> thank you. 

By adding a proper tag you :

1) Assert you did a research in git history/blame to find original
commit.

2) You looked at other similar bugs added by this commit

3) Add meta information in git history to ease backports

4) CC original author to let him/her know he made a mistake and increase
his/her knowledge.

5) Help maintainers



--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Yann Droneaud
Hi Nicolas,

Le mardi 14 avril 2015 à 17:49 +0200, Nicolas Dichtel a écrit :
> Le 14/04/2015 17:44, Honggang LI a écrit :
> > On Tue, Apr 14, 2015 at 08:34:33AM -0700, Eric Dumazet wrote:
> >> On Tue, 2015-04-14 at 23:20 +0800, Honggang Li wrote:
> >>> Starting monitoring for VG vg_rdma01:   3 logical volume(s) in volume
> >>> group "vg_rdma01" monitored
> >>> [  OK  ]
> >>
> >>
> >>> CR2: 0120
> >>> ---[ end trace a8610f6e9640eb85 ]---
> >>>
> >>> Signed-off-by: Honggang Li 
> >>
> >> When was this bug added ?
> >>
> >
> > Sorry, I do not know. I ran into this bug today when I'm tracing a race
> > condition issue related qib. In order to check upstream linux-next fix
> > the race condition or not, I build linux-next-20150414 on two machines. Both
> > machines panic at modprobe ib_ipoib. Do you means I need to report a
> > bug? But I do not know report to who or where.
> 
> Here is the tag:
> Fixes: 5aa7add8f14b ("infiniband/ipoib: implement ndo_get_iflink")
> 

Pardon me, but this patch was never submitted to
linux-rdma@vger.kernel.org for review !?

Regards.

-- 
Yann Droneaud
OPTEYA


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Honggang LI
On Tue, Apr 14, 2015 at 05:49:55PM +0200, Nicolas Dichtel wrote:
> Le 14/04/2015 17:44, Honggang LI a écrit :
> >On Tue, Apr 14, 2015 at 08:34:33AM -0700, Eric Dumazet wrote:
> >>On Tue, 2015-04-14 at 23:20 +0800, Honggang Li wrote:
> >>>Starting monitoring for VG vg_rdma01:   3 logical volume(s) in volume
> >>>group "vg_rdma01" monitored
> >>>[  OK  ]
> >>
> >>
> >>>CR2: 0120
> >>>---[ end trace a8610f6e9640eb85 ]---
> >>>
> >>>Signed-off-by: Honggang Li 
> >>
> >>When was this bug added ?
> >>
> >
> >Sorry, I do not know. I ran into this bug today when I'm tracing a race
> >condition issue related qib. In order to check upstream linux-next fix
> >the race condition or not, I build linux-next-20150414 on two machines. Both
> >machines panic at modprobe ib_ipoib. Do you means I need to report a
> >bug? But I do not know report to who or where.
> 
> Here is the tag:
> Fixes: 5aa7add8f14b ("infiniband/ipoib: implement ndo_get_iflink")
>

thank you. 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Nicolas Dichtel

Le 14/04/2015 17:44, Honggang LI a écrit :

On Tue, Apr 14, 2015 at 08:34:33AM -0700, Eric Dumazet wrote:

On Tue, 2015-04-14 at 23:20 +0800, Honggang Li wrote:

Starting monitoring for VG vg_rdma01:   3 logical volume(s) in volume
group "vg_rdma01" monitored
[  OK  ]




CR2: 0120
---[ end trace a8610f6e9640eb85 ]---

Signed-off-by: Honggang Li 


When was this bug added ?



Sorry, I do not know. I ran into this bug today when I'm tracing a race
condition issue related qib. In order to check upstream linux-next fix
the race condition or not, I build linux-next-20150414 on two machines. Both
machines panic at modprobe ib_ipoib. Do you means I need to report a
bug? But I do not know report to who or where.


Here is the tag:
Fixes: 5aa7add8f14b ("infiniband/ipoib: implement ndo_get_iflink")

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Honggang LI
On Tue, Apr 14, 2015 at 08:34:33AM -0700, Eric Dumazet wrote:
> On Tue, 2015-04-14 at 23:20 +0800, Honggang Li wrote:
> > Starting monitoring for VG vg_rdma01:   3 logical volume(s) in volume
> > group "vg_rdma01" monitored
> > [  OK  ]
> 
> 
> > CR2: 0120
> > ---[ end trace a8610f6e9640eb85 ]---
> > 
> > Signed-off-by: Honggang Li 
> 
> When was this bug added ?
> 

Sorry, I do not know. I ran into this bug today when I'm tracing a race
condition issue related qib. In order to check upstream linux-next fix
the race condition or not, I build linux-next-20150414 on two machines. Both
machines panic at modprobe ib_ipoib. Do you means I need to report a
bug? But I do not know report to who or where.

thanks

> Please add a proper Fixes: tag
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Eric Dumazet
On Tue, 2015-04-14 at 23:20 +0800, Honggang Li wrote:
> Starting monitoring for VG vg_rdma01:   3 logical volume(s) in volume
> group "vg_rdma01" monitored
> [  OK  ]


> CR2: 0120
> ---[ end trace a8610f6e9640eb85 ]---
> 
> Signed-off-by: Honggang Li 

When was this bug added ?

Please add a proper Fixes: tag


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH linux-next 1/4] infiniband/ipoib: fix possible NULL pointer dereference in ipoib_get_iflink

2015-04-14 Thread Honggang Li
Starting monitoring for VG vg_rdma01:   3 logical volume(s) in volume
group "vg_rdma01" monitored
[  OK  ]
Starting cgconfig service: Failed to parse /etc/cgconfig.conf or
/etc/cgconfig.d[FAILED]
Loading OpenIB kernel modules:
BUG: unable to handle kernel NULL pointer dereference at 0120
IP: [] ipoib_get_iflink+0x10/0x20 [ib_ipoib]
PGD 475540067 PUD 473541067 PMD 0
Oops:  [#1] SMP
Modules linked in: ib_ipoib(+) rdma_ucm ib_ucm ib_uverbs ib_umad rdma_cm
ib_cm ib_sa vhost_net macvtap macvlan vhost tun ipmi_devintf sg ipmi_si
ipmi_msghandler serio_raw iTCO_wdt iTCO_vendor_support cdc_ether usbnet
mii bnx2 intel_powerclamp coretemp kvm_intel kvm crc32c_intel
ghash_clmulni_intel aesni_intel ablk_helper cryptd lrw gf128mul
glue_helper aes_x86_64 microcode pcspkr i2c_i801 i2c_core lpc_ich
mfd_core acpi_cpufreq ioatdma i7core_edac edac_core shpchp ext4(E)
jbd2(E) mbcache(E) sd_mod(E) megaraid_sas(E) pata_acpi(E) ata_generic(E)
ata_piix(E) iw_cxgb3(E) cxgb3(E) mdio(E) ib_qib(E) dca(E) ib_mad(E)
iw_cxgb4(E) iw_cm(E) ib_core(E) ib_addr(E) ipv6(E) cxgb4(E) dm_mirror(E)
dm_region_hash(E) dm_log(E) dm_mod(E)
CPU: 6 PID: 2405 Comm: modprobe Tainted: GE
4.0.0-next-20150413 #1
Hardware name: IBM System x3650 M3 -[7945O63]-/00D4062, BIOS
-[D6E157AUS-1.15]- 06/13/2012
task: 880476ad6f00 ti: 88047579c000 task.ti: 88047579c000
RIP: 0010:[]  []
ipoib_get_iflink+0x10/0x20 [ib_ipoib]
RSP: 0018:88047579f9b8  EFLAGS: 00010286
RAX:  RBX: 880476e2a000 RCX: 
RDX: 0004 RSI: 88047579fbb8 RDI: 880476e2a000
RBP: 88047579f9b8 R08: 0660 R09: 88047404f068
R10:  R11:  R12: 8804736bec00
R13: 88047579fbb4 R14: 88047404f000 R15: 0009
FS:  7fc047a2e700() GS:88047fc0()
knlGS:
CS:  0010 DS:  ES:  CR0: 8005003b
CR2: 0120 CR3: 00047541f000 CR4: 06e0
Stack:
 88047579f9c8 814fbfa3 88047579fbe8 81515a15
 0005 880476e2a280 0005 0014
 88047579fa48 8150a577  8804
Call Trace:
 [] dev_get_iflink+0x23/0x40
 [] rtnl_fill_ifinfo+0x255/0xce0
 [] ? __hw_addr_create_ex+0x97/0xc0
 [] ? _raw_spin_unlock_bh+0x1b/0x20
 [] ? __dev_mc_add+0x75/0x90
 [] ? igmp6_group_added+0x5c/0x130 [ipv6]
 [] ? __kmalloc_node_track_caller+0x3c/0x50
 [] ? __kmalloc_reserve+0x3b/0xa0
 [] ? __alloc_skb+0xa8/0x1f0
 [] rtmsg_ifinfo_build_skb+0x83/0xe0
 [] ? raw_notifier_call_chain+0x16/0x20
 [] rtmsg_ifinfo+0x21/0x40
 [] register_netdevice+0x38f/0x400
 [] register_netdev+0x1e/0x30
 [] ipoib_add_port.clone.0+0x214/0x390 [ib_ipoib]
 [] ipoib_add_one+0xc7/0x110 [ib_ipoib]
 [] ib_register_client+0x7d/0xa0 [ib_core]
 [] ? 0xa06ce000
 [] ipoib_init_module+0xf2/0x13c [ib_ipoib]
 [] do_one_initcall+0xb7/0x1d0
 [] do_init_module+0x69/0x200
 [] load_module+0x5b5/0x730
 [] ? mod_sysfs_teardown+0x150/0x150
 [] ? __vmalloc+0x22/0x30
 [] ? module_sect_show+0x30/0x30
 [] SyS_init_module+0x94/0xc0
 [] system_call_fastpath+0x12/0x6a
Code: 66 66 66 90 b9 1e 00 00 00 48 89 f0 48 8d 77 08 48 89 c7 f3 48 a5
c9 c3 0f 1f 00 55 48 89 e5 66 66 66 66 90 48 8b 87 e8 13 00 00 <8b> 80
20 01 00 00 c9 c3 0f 1f 84 00 00 00 00 00 55 48 89 e5 66
RIP  [] ipoib_get_iflink+0x10/0x20 [ib_ipoib]
 RSP 
CR2: 0120
---[ end trace a8610f6e9640eb85 ]---

Signed-off-by: Honggang Li 
---
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c 
b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 657b89b..11ea6e2 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -846,7 +846,10 @@ static int ipoib_get_iflink(const struct net_device *dev)
 {
struct ipoib_dev_priv *priv = netdev_priv(dev);
 
-   return priv->parent->ifindex;
+   if (priv && priv->parent)
+   return priv->parent->ifindex;
+   else
+   return 0;
 }
 
 static u32 ipoib_addr_hash(struct ipoib_neigh_hash *htbl, u8 *daddr)
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html