netdev - RE: [PATCH net-next RFC] ipv4 neigh: match add/del dev lookup behaviour to SIOCSARP/SIOCDARP

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: <9B0331B6EBBD0E4684FBFAEDA55776F9189654A5@HASMSX110.ger.corp.intel.com>
Date:	Tue, 24 Feb 2015 14:01:30 +0000
From:	"Rosen, Rami" <rami.rosen@...el.com>
To:	"roopa@...ulusnetworks.com" <roopa@...ulusnetworks.com>,
	"hadi@...atatu.com" <hadi@...atatu.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"hannes@...essinduktion.org" <hannes@...essinduktion.org>
Subject: RE: [PATCH net-next RFC] ipv4 neigh: match add/del dev lookup
 behaviour to SIOCSARP/SIOCDARP

Hi,

@@ -1692,24 +1703,31 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh)
 		goto out;

 	ndm = nlmsg_data(nlh);
+	tbl = neigh_find_table(ndm->ndm_family);
+	if (tbl == NULL)
+		return -EAFNOSUPPORT;
+
+	if (nla_len(tb[NDA_DST]) < tbl->key_len)
+		goto out;
+
+	dst = nla_data(tb[NDA_DST]);
 	if (ndm->ndm_ifindex) {
 		dev = __dev_get_by_index(net, ndm->ndm_ifindex);
 		if (dev == NULL) {
 			err = -ENODEV;
 			goto out;
 		}

If ip_route_output() fails, then the neigh_add() method will return -EINVAL (as it was initialized thus); or, in cases when the NTF_PROXY flag is set, it can return 0 or ENOBUFS, according to the result of the pneigh_lookup(), which is called immediately subsequently in this path. But in fact __ip_route_output_key(), which is invoked by ip_route_output(), can return different error codes, like -EINVAL,-ENODEV, -ENETUNREACH and more, via ERR_PTR. 

 So maybe better is (in order to reflect the cause of the error):
+		if (!IS_ERR(rt)) {
+			dev = rt->dst.dev;
+			ip_rt_put(rt);
+		} else
                                               return PTR_ERR(rt);

We can exit the method at this point with "return PTR_ERR(rt)" because we are assured at this point that dev is NULL.

Error handling in neigh_delete() method maybe also be better changed accordingly.

Regards,
Rami Rosen
Intel Corporation

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