diff mbox

[linux-next,3/4] macvlan: fix possible NULL pointer dereference in macvlan_dev_get_iflink

Message ID 1429024817-21561-4-git-send-email-honli@redhat.com (mailing list archive)
State Rejected
Headers show

Commit Message

Honggang LI April 14, 2015, 3:20 p.m. UTC
Signed-off-by: Honggang Li <honli@redhat.com>
---
 drivers/net/macvlan.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Patrick McHardy April 14, 2015, 3:26 p.m. UTC | #1
On 14.04, Honggang Li wrote:
> Signed-off-by: Honggang Li <honli@redhat.com>
> ---
>  drivers/net/macvlan.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index b227a13..1e59f39 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -998,7 +998,9 @@ static int macvlan_dev_get_iflink(const struct net_device *dev)
>  {
>  	struct macvlan_dev *vlan = netdev_priv(dev);
>  
> -	return vlan->lowerdev->ifindex;
> +	if (vlan && vlan->lowerdev)
> +		return vlan->lowerdev->ifindex;

That is completely useless. vlan (=netdev_priv) can not be NULL as
netdev_priv() never returns NULL and vlan->lowerdev is always valid
because a macvlan wouldn't make much sense otherwise.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Honggang LI April 14, 2015, 3:32 p.m. UTC | #2
On Tue, Apr 14, 2015 at 04:26:27PM +0100, Patrick McHardy wrote:
> 
> That is completely useless. vlan (=netdev_priv) can not be NULL as
> netdev_priv() never returns NULL and vlan->lowerdev is always valid
> because a macvlan wouldn't make much sense otherwise.

OK, please drop this patch.

thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Dichtel April 14, 2015, 3:35 p.m. UTC | #3
Le 14/04/2015 17:26, Patrick McHardy a écrit :
> On 14.04, Honggang Li wrote:
[snip]
>> -	return vlan->lowerdev->ifindex;
>> +	if (vlan && vlan->lowerdev)
>> +		return vlan->lowerdev->ifindex;
>
> That is completely useless. vlan (=netdev_priv) can not be NULL as
> netdev_priv() never returns NULL and vlan->lowerdev is always valid
> because a macvlan wouldn't make much sense otherwise.
>
And I suspect that it is the same for ipvlan and dsa.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Patrick McHardy April 14, 2015, 3:35 p.m. UTC | #4
On 14.04, Honggang LI wrote:
> On Tue, Apr 14, 2015 at 04:26:27PM +0100, Patrick McHardy wrote:
> > 
> > That is completely useless. vlan (=netdev_priv) can not be NULL as
> > netdev_priv() never returns NULL and vlan->lowerdev is always valid
> > because a macvlan wouldn't make much sense otherwise.
> 
> OK, please drop this patch.

Well, the fact that netdev_priv never return NULL implies all your
patches need to be redone.

And I suggest you check whether this condition can actually happen.
Just because it can in one driver says nothing at all about others.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Lunn April 14, 2015, 3:37 p.m. UTC | #5
On Tue, Apr 14, 2015 at 05:35:12PM +0200, Nicolas Dichtel wrote:
> Le 14/04/2015 17:26, Patrick McHardy a écrit :
> >On 14.04, Honggang Li wrote:
> [snip]
> >>-	return vlan->lowerdev->ifindex;
> >>+	if (vlan && vlan->lowerdev)
> >>+		return vlan->lowerdev->ifindex;
> >
> >That is completely useless. vlan (=netdev_priv) can not be NULL as
> >netdev_priv() never returns NULL and vlan->lowerdev is always valid
> >because a macvlan wouldn't make much sense otherwise.
> >
> And I suspect that it is the same for ipvlan and dsa.

I agree about DSA. I don't see any way this could happen.

  Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Honggang LI April 14, 2015, 3:46 p.m. UTC | #6
On Tue, Apr 14, 2015 at 05:37:57PM +0200, Andrew Lunn wrote:
> > >
> > And I suspect that it is the same for ipvlan and dsa.
> 
> I agree about DSA. I don't see any way this could happen.
> 
>   Andrew

I only keep the ipoib patch and drop the rest patches.

thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller April 14, 2015, 5:47 p.m. UTC | #7
From: Honggang LI <honli@redhat.com>
Date: Tue, 14 Apr 2015 23:32:39 +0800

> On Tue, Apr 14, 2015 at 04:26:27PM +0100, Patrick McHardy wrote:
>> 
>> That is completely useless. vlan (=netdev_priv) can not be NULL as
>> netdev_priv() never returns NULL and vlan->lowerdev is always valid
>> because a macvlan wouldn't make much sense otherwise.
> 
> OK, please drop this patch.

That's not how this works.

When a patch series needs any chnages, you must make a fresh, new
complete submission of the entire patch series.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index b227a13..1e59f39 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -998,7 +998,9 @@  static int macvlan_dev_get_iflink(const struct net_device *dev)
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
 
-	return vlan->lowerdev->ifindex;
+	if (vlan && vlan->lowerdev)
+		return vlan->lowerdev->ifindex;
+	return 0;
 }
 
 static const struct ethtool_ops macvlan_ethtool_ops = {