netdev - Re: v3 [PATCH net-next 5/7] ipv4: Check return of dev_set

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: <4873364F.1030709@cn.fujitsu.com>
Date:	Tue, 08 Jul 2008 17:41:35 +0800
From:	Wang Chen <wangchen@...fujitsu.com>
To:	Patrick McHardy <kaber@...sh.net>
CC:	"David S. Miller" <davem@...emloft.net>,
	NETDEV <netdev@...r.kernel.org>
Subject: Re: v3 [PATCH net-next 5/7] ipv4: Check return of dev_set_allmulti

Patrick McHardy said the following on 2008-7-7 19:22:
>>      case 0:
>>          dev = ip_dev_find(&init_net, vifc->vifc_lcl_addr.s_addr);
>>          if (!dev)
>>              return -EADDRNOTAVAIL;
>>          dev_put(dev);
>> +        err = dev_set_allmulti(dev, 1);
>> +        if (err)
>> +            return err;
> 
> Also looks like a use after free, but again, one that is
> already present without your patch.
> 

Here is the patch for fixing use after free.
It fixes both ipv4 and ipv6 side and on top of my patches.
This patch will be the 6/8 of the series.

I will wait for Patrick's ack and resend the whole series again.

Signed-off-by: Wang Chen <wangchen@...fujitsu.com>
---
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index a55a23a..23fa3f3 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -441,8 +441,10 @@ static int vif_add(struct vifctl *vifc, int mrtsock)
 		dev = ipmr_reg_vif();
 		if (!dev)
 			return -ENOBUFS;
+		dev_hold(dev);
 		err = dev_set_allmulti(dev, 1);
 		if (err) {
+			dev_put(dev);
 			unregister_netdevice(dev);
 			return err;
 		}
@@ -452,8 +454,10 @@ static int vif_add(struct vifctl *vifc, int mrtsock)
 		dev = ipmr_new_tunnel(vifc);
 		if (!dev)
 			return -ENOBUFS;
+		dev_hold(dev);
 		err = dev_set_allmulti(dev, 1);
 		if (err) {
+			dev_put(dev);
 			ipmr_del_tunnel(dev, vifc);
 			return err;
 		}
@@ -462,10 +466,11 @@ static int vif_add(struct vifctl *vifc, int mrtsock)
 		dev = ip_dev_find(&init_net, vifc->vifc_lcl_addr.s_addr);
 		if (!dev)
 			return -EADDRNOTAVAIL;
-		dev_put(dev);
 		err = dev_set_allmulti(dev, 1);
-		if (err)
+		if (err) {
+			dev_put(dev);
 			return err;
+		}
 		break;
 	default:
 		return -EINVAL;
@@ -496,7 +501,6 @@ static int vif_add(struct vifctl *vifc, int mrtsock)
 
 	/* And finish update writing critical data */
 	write_lock_bh(&mrt_lock);
-	dev_hold(dev);
 	v->dev=dev;
 #ifdef CONFIG_IP_PIMSM
 	if (v->flags&VIFF_REGISTER)
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 6cd286d..a9bd74d 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -621,8 +621,10 @@ static int mif6_add(struct mif6ctl *vifc, int mrtsock)
 		dev = ip6mr_reg_vif();
 		if (!dev)
 			return -ENOBUFS;
+		dev_hold(dev);
 		err = dev_set_allmulti(dev, 1);
 		if (err) {
+			dev_put(dev);
 			unregister_netdevice(dev);
 			return err;
 		}
@@ -632,10 +634,11 @@ static int mif6_add(struct mif6ctl *vifc, int mrtsock)
 		dev = dev_get_by_index(&init_net, vifc->mif6c_pifi);
 		if (!dev)
 			return -EADDRNOTAVAIL;
-		dev_put(dev);
 		err = dev_set_allmulti(dev, 1);
-		if (err)
+		if (err) {
+			dev_put(dev);
 			return err;
+		}
 		break;
 	default:
 		return -EINVAL;
@@ -659,7 +662,6 @@ static int mif6_add(struct mif6ctl *vifc, int mrtsock)
 
 	/* And finish update writing critical data */
 	write_lock_bh(&mrt_lock);
-	dev_hold(dev);
 	v->dev = dev;
 #ifdef CONFIG_IPV6_PIMSM_V2
 	if (v->flags & MIFF_REGISTER)

--
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