[PKT_SCHED]: Add rtattr_strlcpy, use it where appropriate

Add rtattr_strlcpy to handle unterminated strings. The destination
is nulled out entirely to avoid possible leaks when dumping. The
return value can be checked for >= size to detect truncated strings.
Currently strings equal to the size of the destination are accepted
everywhere even if not null-terminated. Sometimes they are silently
truncated, sometimes the unterminated string is used.

Signed-off-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: David S. Miller <davem@davemloft.net>

8 files changed, 37 insertions, 42 deletions

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d97b6a60aeb0a1..d69ad90e581165 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -124,6 +124,21 @@ void __rta_fill(struct sk_buff *skb, int attrtype, int attrlen, const void *data
 	memcpy(RTA_DATA(rta), data, attrlen);
 }
 
+size_t rtattr_strlcpy(char *dest, const struct rtattr *rta, size_t size)
+{
+	size_t ret = RTA_PAYLOAD(rta);
+	char *src = RTA_DATA(rta);
+
+	if (ret > 0 && src[ret - 1] == '\0')
+		ret--;
+	if (size > 0) {
+		size_t len = (ret >= size) ? size - 1 : ret;
+		memset(dest, 0, size);
+		memcpy(dest, src, len);
+	}
+	return ret;
+}
+
 int rtnetlink_send(struct sk_buff *skb, u32 pid, unsigned group, int echo)
 {
 	int err = 0;
@@ -277,13 +292,9 @@ static int do_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 	else if (ida[IFLA_IFNAME - 1]) {
 		char ifname[IFNAMSIZ];
 
-		if (RTA_PAYLOAD(ida[IFLA_IFNAME - 1]) > RTA_ALIGN(sizeof(ifname)))
+		if (rtattr_strlcpy(ifname, ida[IFLA_IFNAME - 1],
+		                   IFNAMSIZ) >= IFNAMSIZ)
 			return -EINVAL;
-
-		memset(ifname, 0, sizeof(ifname));
-		memcpy(ifname, RTA_DATA(ida[IFLA_IFNAME - 1]),
-			RTA_PAYLOAD(ida[IFLA_IFNAME - 1]));
-		ifname[IFNAMSIZ - 1] = '\0';
 		dev = dev_get_by_name(ifname);
 	} else
 		return -EINVAL;
@@ -381,16 +392,10 @@ static int do_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 	if (ifm->ifi_index >= 0 && ida[IFLA_IFNAME - 1]) {
 		char ifname[IFNAMSIZ];
 
-		if (RTA_PAYLOAD(ida[IFLA_IFNAME - 1]) > RTA_ALIGN(sizeof(ifname)))
+		if (rtattr_strlcpy(ifname, ida[IFLA_IFNAME - 1],
+		                   IFNAMSIZ) >= IFNAMSIZ)
 			goto out;
-
-		memset(ifname, 0, sizeof(ifname));
-		memcpy(ifname, RTA_DATA(ida[IFLA_IFNAME - 1]),
-			RTA_PAYLOAD(ida[IFLA_IFNAME - 1]));
-		ifname[IFNAMSIZ - 1] = '\0';
-
 		err = dev_change_name(dev, ifname);
-
 		if (err)
 			goto out;
 	}
@@ -695,6 +700,7 @@ void __init rtnetlink_init(void)
 }
 
 EXPORT_SYMBOL(__rta_fill);
+EXPORT_SYMBOL(rtattr_strlcpy);
 EXPORT_SYMBOL(rtattr_parse);
 EXPORT_SYMBOL(rtnetlink_links);
 EXPORT_SYMBOL(rtnetlink_put_metrics);
diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
index aba84350596c44..72a18e22b6233a 100644
--- a/net/decnet/dn_dev.c
+++ b/net/decnet/dn_dev.c
@@ -705,7 +705,7 @@ static int dn_dev_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh, void *a
 	ifa->ifa_scope = ifm->ifa_scope;
 	ifa->ifa_dev = dn_db;
 	if (rta[IFA_LABEL-1])
-		memcpy(ifa->ifa_label, RTA_DATA(rta[IFA_LABEL-1]), IFNAMSIZ);
+		rtattr_strlcpy(ifa->ifa_label, rta[IFA_LABEL-1], IFNAMSIZ);
 	else
 		memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
 
diff --git a/net/decnet/dn_rules.c b/net/decnet/dn_rules.c
index 17cc9c356ba2a9..00bb03e52ba859 100644
--- a/net/decnet/dn_rules.c
+++ b/net/decnet/dn_rules.c
@@ -170,8 +170,7 @@ int dn_fib_rtm_newrule(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 	new_r->r_table = table_id;
 	if (rta[RTA_IIF-1]) {
 		struct net_device *dev;
-		memcpy(new_r->r_ifname, RTA_DATA(rta[RTA_IIF-1]), IFNAMSIZ);
-		new_r->r_ifname[IFNAMSIZ-1] = 0;
+		rtattr_strlcpy(new_r->r_ifname, rta[RTA_IIF-1], IFNAMSIZ);
 		new_r->r_ifindex = -1;
 		dev = dev_get_by_name(new_r->r_ifname);
 		if (dev) {
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 7b6054d4927102..587808ca1b5b82 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -456,7 +456,7 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg
 	in_dev_hold(in_dev);
 	ifa->ifa_dev   = in_dev;
 	if (rta[IFA_LABEL - 1])
-		memcpy(ifa->ifa_label, RTA_DATA(rta[IFA_LABEL - 1]), IFNAMSIZ);
+		rtattr_strlcpy(ifa->ifa_label, rta[IFA_LABEL - 1], IFNAMSIZ);
 	else
 		memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
 
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index f6675eef21cf1b..ddd75cbe4f96bf 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -209,8 +209,7 @@ int inet_rtm_newrule(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
 	new_r->r_table = table_id;
 	if (rta[RTA_IIF-1]) {
 		struct net_device *dev;
-		memcpy(new_r->r_ifname, RTA_DATA(rta[RTA_IIF-1]), IFNAMSIZ);
-		new_r->r_ifname[IFNAMSIZ-1] = 0;
+		rtattr_strlcpy(new_r->r_ifname, rta[RTA_IIF-1], IFNAMSIZ);
 		new_r->r_ifindex = -1;
 		dev = __dev_get_by_name(new_r->r_ifname);
 		if (dev)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 7c2fd5ce2bc9a4..6c873708cc62b0 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -272,7 +272,7 @@ struct tc_action *tcf_action_init_1(struct rtattr *rta, struct rtattr *est,
 {
 	struct tc_action *a;
 	struct tc_action_ops *a_o;
-	char act_name[4 + IFNAMSIZ + 1];
+	char act_name[IFNAMSIZ];
 	struct rtattr *tb[TCA_ACT_MAX+1];
 	struct rtattr *kind;
 
@@ -284,12 +284,9 @@ struct tc_action *tcf_action_init_1(struct rtattr *rta, struct rtattr *est,
 			goto err_out;
 		kind = tb[TCA_ACT_KIND-1];
 		if (kind != NULL) {
-			sprintf(act_name, "%s", (char*)RTA_DATA(kind));
-			if (RTA_PAYLOAD(kind) >= IFNAMSIZ) {
-				printk("Action %s bad\n",
-				        (char*)RTA_DATA(kind));
+			if (rtattr_strlcpy(act_name, kind,
+			                   IFNAMSIZ) >= IFNAMSIZ)
 				goto err_out;
-			}
 		} else {
 			printk("Action bad kind\n");
 			goto err_out;
@@ -492,7 +489,7 @@ static int tcf_action_get_1(struct rtattr *rta, struct tc_action *a,
                             struct nlmsghdr *n, u32 pid)
 {
 	struct tc_action_ops *a_o;
-	char act_name[4 + IFNAMSIZ + 1];
+	char act_name[IFNAMSIZ];
 	struct rtattr *tb[TCA_ACT_MAX+1];
 	struct rtattr *kind;
 	int index;
@@ -502,12 +499,8 @@ static int tcf_action_get_1(struct rtattr *rta, struct tc_action *a,
 		goto err_out;
 	kind = tb[TCA_ACT_KIND-1];
 	if (kind != NULL) {
-		sprintf(act_name, "%s", (char*)RTA_DATA(kind));
-		if (RTA_PAYLOAD(kind) >= IFNAMSIZ) {
-			printk("tcf_action_get_1: action %s bad\n",
-			       (char*)RTA_DATA(kind));
+		if (rtattr_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ)
 			goto err_out;
-		}
 	} else {
 		printk("tcf_action_get_1: action bad kind\n");
 		goto err_out;
@@ -562,16 +555,12 @@ static void cleanup_a(struct tc_action *act)
 
 static struct tc_action_ops *get_ao(struct rtattr *kind, struct tc_action *a)
 {
-	char act_name[4 + IFNAMSIZ + 1];
+	char act_name[IFNAMSIZ];
 	struct tc_action_ops *a_o;
 
 	if (kind != NULL) {
-		sprintf(act_name, "%s", (char*)RTA_DATA(kind));
-		if (RTA_PAYLOAD(kind) >= IFNAMSIZ) {
-			printk("get_ao: action %s bad\n",
-			       (char*)RTA_DATA(kind));
+		if (rtattr_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ)
 			return NULL;
-		}
 	} else {
 		printk("get_ao: action bad kind\n");
 		return NULL;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 2bc0fe12737de0..c2e298468d27c8 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -215,9 +215,10 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 #ifdef CONFIG_KMOD
 		if (tp_ops==NULL && tca[TCA_KIND-1] != NULL) {
 			struct rtattr *kind = tca[TCA_KIND-1];
+			char name[IFNAMSIZ];
 
-			if (RTA_PAYLOAD(kind) <= IFNAMSIZ) {
-				request_module("cls_%s", (char*)RTA_DATA(kind));
+			if (rtattr_strlcpy(name, kind, IFNAMSIZ) < IFNAMSIZ) {
+				request_module("cls_%s", name);
 				tp_ops = tcf_proto_lookup_ops(kind);
 			}
 		}
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index a8570f361d6d88..26b33113568e22 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -407,8 +407,9 @@ qdisc_create(struct net_device *dev, u32 handle, struct rtattr **tca, int *errp)
 	ops = qdisc_lookup_ops(kind);
 #ifdef CONFIG_KMOD
 	if (ops==NULL && tca[TCA_KIND-1] != NULL) {
-		if (RTA_PAYLOAD(kind) <= IFNAMSIZ) {
-			request_module("sch_%s", (char*)RTA_DATA(kind));
+		char name[IFNAMSIZ];
+		if (rtattr_strlcpy(name, kind, IFNAMSIZ) < IFNAMSIZ) {
+			request_module("sch_%s", name);
 			ops = qdisc_lookup_ops(kind);
 		}
 	}