[PATCH v2] tipc: fix potential null pointer dereferences in some compat functions

[PATCH v2] tipc: fix potential null pointer dereferences in some compat functions

[Baozeng Ding]

Before calling the nla_parse_nested function, make sure the pointer to the
attribute is not null. This patch fixes several potential null pointer
dereference vulnerabilities in the tipc netlink functions.

Signed-off-by: Baozeng Ding <sploving1@gmail.com>
---
v2: declare local variable as reverse christmas tree format and make the commit
log fit in 80 columns
---
 net/tipc/netlink_compat.c | 111 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 93 insertions(+), 18 deletions(-)

diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
index d7d050f..72a1c8f 100644
--- a/net/tipc/netlink_compat.c
+++ b/net/tipc/netlink_compat.c
@@ -346,9 +346,15 @@ static int tipc_nl_compat_bearer_dump(struct tipc_nl_compat_msg *msg,
 				      struct nlattr **attrs)
 {
 	struct nlattr *bearer[TIPC_NLA_BEARER_MAX + 1];
+	int err;
+
+	if (!attrs[TIPC_NLA_BEARER])
+		return -EINVAL;
 
-	nla_parse_nested(bearer, TIPC_NLA_BEARER_MAX, attrs[TIPC_NLA_BEARER],
-			 NULL);
+	err = nla_parse_nested(bearer, TIPC_NLA_BEARER_MAX,
+			       attrs[TIPC_NLA_BEARER], NULL);
+	if (err)
+		return err;
 
 	return tipc_add_tlv(msg->rep, TIPC_TLV_BEARER_NAME,
 			    nla_data(bearer[TIPC_NLA_BEARER_NAME]),
@@ -460,14 +466,31 @@ static int tipc_nl_compat_link_stat_dump(struct tipc_nl_compat_msg *msg,
 	struct nlattr *link[TIPC_NLA_LINK_MAX + 1];
 	struct nlattr *prop[TIPC_NLA_PROP_MAX + 1];
 	struct nlattr *stats[TIPC_NLA_STATS_MAX + 1];
+	int err;
 
-	nla_parse_nested(link, TIPC_NLA_LINK_MAX, attrs[TIPC_NLA_LINK], NULL);
+	if (!attrs[TIPC_NLA_LINK])
+		return -EINVAL;
 
-	nla_parse_nested(prop, TIPC_NLA_PROP_MAX, link[TIPC_NLA_LINK_PROP],
-			 NULL);
+	err = nla_parse_nested(link, TIPC_NLA_LINK_MAX, attrs[TIPC_NLA_LINK],
+			       NULL);
+	if (err)
+		return err;
+
+	if (!link[TIPC_NLA_LINK_PROP])
+		return -EINVAL;
 
-	nla_parse_nested(stats, TIPC_NLA_STATS_MAX, link[TIPC_NLA_LINK_STATS],
-			 NULL);
+	err = nla_parse_nested(prop, TIPC_NLA_PROP_MAX,
+			       link[TIPC_NLA_LINK_PROP], NULL);
+	if (err)
+		return err;
+
+	if (!link[TIPC_NLA_LINK_STATS])
+		return -EINVAL;
+
+	err = nla_parse_nested(stats, TIPC_NLA_STATS_MAX,
+			       link[TIPC_NLA_LINK_STATS], NULL);
+	if (err)
+		return err;
 
 	name = (char *)TLV_DATA(msg->req);
 	if (strcmp(name, nla_data(link[TIPC_NLA_LINK_NAME])) != 0)
@@ -569,8 +592,15 @@ static int tipc_nl_compat_link_dump(struct tipc_nl_compat_msg *msg,
 {
 	struct nlattr *link[TIPC_NLA_LINK_MAX + 1];
 	struct tipc_link_info link_info;
+	int err;
 
-	nla_parse_nested(link, TIPC_NLA_LINK_MAX, attrs[TIPC_NLA_LINK], NULL);
+	if (!attrs[TIPC_NLA_LINK])
+		return -EINVAL;
+
+	err = nla_parse_nested(link, TIPC_NLA_LINK_MAX, attrs[TIPC_NLA_LINK],
+			       NULL);
+	if (err)
+		return err;
 
 	link_info.dest = nla_get_flag(link[TIPC_NLA_LINK_DEST]);
 	link_info.up = htonl(nla_get_flag(link[TIPC_NLA_LINK_UP]));
@@ -758,12 +788,23 @@ static int tipc_nl_compat_name_table_dump(struct tipc_nl_compat_msg *msg,
 	u32 node, depth, type, lowbound, upbound;
 	static const char * const scope_str[] = {"", " zone", " cluster",
 						 " node"};
+	int err;
 
-	nla_parse_nested(nt, TIPC_NLA_NAME_TABLE_MAX,
-			 attrs[TIPC_NLA_NAME_TABLE], NULL);
+	if (!attrs[TIPC_NLA_NAME_TABLE])
+		return -EINVAL;
 
-	nla_parse_nested(publ, TIPC_NLA_PUBL_MAX, nt[TIPC_NLA_NAME_TABLE_PUBL],
-			 NULL);
+	err = nla_parse_nested(nt, TIPC_NLA_NAME_TABLE_MAX,
+			       attrs[TIPC_NLA_NAME_TABLE], NULL);
+	if (err)
+		return err;
+
+	if (!nt[TIPC_NLA_NAME_TABLE_PUBL])
+		return -EINVAL;
+
+	err = nla_parse_nested(publ, TIPC_NLA_PUBL_MAX,
+			       nt[TIPC_NLA_NAME_TABLE_PUBL], NULL);
+	if (err)
+		return err;
 
 	ntq = (struct tipc_name_table_query *)TLV_DATA(msg->req);
 
@@ -815,8 +856,15 @@ static int __tipc_nl_compat_publ_dump(struct tipc_nl_compat_msg *msg,
 {
 	u32 type, lower, upper;
 	struct nlattr *publ[TIPC_NLA_PUBL_MAX + 1];
+	int err;
 
-	nla_parse_nested(publ, TIPC_NLA_PUBL_MAX, attrs[TIPC_NLA_PUBL], NULL);
+	if (!attrs[TIPC_NLA_PUBL])
+		return -EINVAL;
+
+	err = nla_parse_nested(publ, TIPC_NLA_PUBL_MAX, attrs[TIPC_NLA_PUBL],
+			       NULL);
+	if (err)
+		return err;
 
 	type = nla_get_u32(publ[TIPC_NLA_PUBL_TYPE]);
 	lower = nla_get_u32(publ[TIPC_NLA_PUBL_LOWER]);
@@ -876,7 +924,13 @@ static int tipc_nl_compat_sk_dump(struct tipc_nl_compat_msg *msg,
 	u32 sock_ref;
 	struct nlattr *sock[TIPC_NLA_SOCK_MAX + 1];
 
-	nla_parse_nested(sock, TIPC_NLA_SOCK_MAX, attrs[TIPC_NLA_SOCK], NULL);
+	if (!attrs[TIPC_NLA_SOCK])
+		return -EINVAL;
+
+	err = nla_parse_nested(sock, TIPC_NLA_SOCK_MAX, attrs[TIPC_NLA_SOCK],
+			       NULL);
+	if (err)
+		return err;
 
 	sock_ref = nla_get_u32(sock[TIPC_NLA_SOCK_REF]);
 	tipc_tlv_sprintf(msg->rep, "%u:", sock_ref);
@@ -917,9 +971,15 @@ static int tipc_nl_compat_media_dump(struct tipc_nl_compat_msg *msg,
 				     struct nlattr **attrs)
 {
 	struct nlattr *media[TIPC_NLA_MEDIA_MAX + 1];
+	int err;
+
+	if (!attrs[TIPC_NLA_MEDIA])
+		return -EINVAL;
 
-	nla_parse_nested(media, TIPC_NLA_MEDIA_MAX, attrs[TIPC_NLA_MEDIA],
-			 NULL);
+	err = nla_parse_nested(media, TIPC_NLA_MEDIA_MAX, attrs[TIPC_NLA_MEDIA],
+			       NULL);
+	if (err)
+		return err;
 
 	return tipc_add_tlv(msg->rep, TIPC_TLV_MEDIA_NAME,
 			    nla_data(media[TIPC_NLA_MEDIA_NAME]),
@@ -931,8 +991,15 @@ static int tipc_nl_compat_node_dump(struct tipc_nl_compat_msg *msg,
 {
 	struct tipc_node_info node_info;
 	struct nlattr *node[TIPC_NLA_NODE_MAX + 1];
+	int err;
 
-	nla_parse_nested(node, TIPC_NLA_NODE_MAX, attrs[TIPC_NLA_NODE], NULL);
+	if (!attrs[TIPC_NLA_NODE])
+		return -EINVAL;
+
+	err = nla_parse_nested(node, TIPC_NLA_NODE_MAX, attrs[TIPC_NLA_NODE],
+			       NULL);
+	if (err)
+		return err;
 
 	node_info.addr = htonl(nla_get_u32(node[TIPC_NLA_NODE_ADDR]));
 	node_info.up = htonl(nla_get_flag(node[TIPC_NLA_NODE_UP]));
@@ -971,8 +1038,16 @@ static int tipc_nl_compat_net_dump(struct tipc_nl_compat_msg *msg,
 {
 	__be32 id;
 	struct nlattr *net[TIPC_NLA_NET_MAX + 1];
+	int err;
+
+	if (!attrs[TIPC_NLA_NET])
+		return -EINVAL;
+
+	err = nla_parse_nested(net, TIPC_NLA_NET_MAX, attrs[TIPC_NLA_NET],
+			       NULL);
+	if (err)
+		return err;
 
-	nla_parse_nested(net, TIPC_NLA_NET_MAX, attrs[TIPC_NLA_NET], NULL);
 	id = htonl(nla_get_u32(net[TIPC_NLA_NET_ID]));
 
 	return tipc_add_tlv(msg->rep, TIPC_TLV_UNSIGNED, &id, sizeof(id));
-- 
1.9.1

Re: [PATCH v2] tipc: fix potential null pointer dereferences in some compat functions

[David Miller]

From: Baozeng Ding <sploving1@gmail.com>
Date: Tue, 24 May 2016 22:33:24 +0800

> Before calling the nla_parse_nested function, make sure the pointer to the
> attribute is not null. This patch fixes several potential null pointer
> dereference vulnerabilities in the tipc netlink functions.
> 
> Signed-off-by: Baozeng Ding <sploving1@gmail.com>
> ---
> v2: declare local variable as reverse christmas tree format and make the commit
> log fit in 80 columns

Looks good, applied, thanks.

[PATCH] tipc: fix potential null pointer dereference in the nla_data function

[Baozeng Ding]

Before calling the nla_data function, make sure the argument is not null.
Fix potential null pointer dereference vulnerability for this.

Signed-off-by: Baozeng Ding <sploving1@gmail.com>
---
 net/tipc/netlink_compat.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
index f795b1d..efbba26 100644
--- a/net/tipc/netlink_compat.c
+++ b/net/tipc/netlink_compat.c
@@ -356,6 +356,9 @@ static int tipc_nl_compat_bearer_dump(struct tipc_nl_compat_msg *msg,
 	if (err)
 		return err;
 
+	if (!bearer[TIPC_NLA_BEARER_NAME])
+		return -EINVAL;
+
 	return tipc_add_tlv(msg->rep, TIPC_TLV_BEARER_NAME,
 			    nla_data(bearer[TIPC_NLA_BEARER_NAME]),
 			    nla_len(bearer[TIPC_NLA_BEARER_NAME]));
@@ -492,6 +495,9 @@ static int tipc_nl_compat_link_stat_dump(struct tipc_nl_compat_msg *msg,
 	if (err)
 		return err;
 
+	if (!link[TIPC_NLA_LINK_NAME])
+		return -EINVAL;
+
 	name = (char *)TLV_DATA(msg->req);
 	if (strcmp(name, nla_data(link[TIPC_NLA_LINK_NAME])) != 0)
 		return 0;
@@ -602,6 +608,9 @@ static int tipc_nl_compat_link_dump(struct tipc_nl_compat_msg *msg,
 	if (err)
 		return err;
 
+	if (!link[TIPC_NLA_LINK_NAME])
+		return -EINVAL;
+
 	link_info.dest = nla_get_flag(link[TIPC_NLA_LINK_DEST]);
 	link_info.up = htonl(nla_get_flag(link[TIPC_NLA_LINK_UP]));
 	strcpy(link_info.str, nla_data(link[TIPC_NLA_LINK_NAME]));
@@ -981,6 +990,9 @@ static int tipc_nl_compat_media_dump(struct tipc_nl_compat_msg *msg,
 	if (err)
 		return err;
 
+	if (!media[TIPC_NLA_MEDIA_NAME])
+		return -EINVAL;
+
 	return tipc_add_tlv(msg->rep, TIPC_TLV_MEDIA_NAME,
 			    nla_data(media[TIPC_NLA_MEDIA_NAME]),
 			    nla_len(media[TIPC_NLA_MEDIA_NAME]));
-- 
1.9.1

Re: [PATCH] tipc: fix potential null pointer dereference in the nla_data function

[David Miller]

From: Baozeng Ding <sploving1@gmail.com>
Date: Fri, 10 Jun 2016 10:26:59 +0800

> Before calling the nla_data function, make sure the argument is not null.
> Fix potential null pointer dereference vulnerability for this.
> 
> Signed-off-by: Baozeng Ding <sploving1@gmail.com>

TIPC maintainers, please review.

Re: [PATCH] tipc: fix potential null pointer dereference in the nla_data function

[Erik Hugne]

On Jun 11, 2016 08:24, "David Miller" <davem@davemloft.net> wrote:
>
> From: Baozeng Ding <sploving1@gmail.com>
> Date: Fri, 10 Jun 2016 10:26:59 +0800
>
> > Before calling the nla_data function, make sure the argument is not
null.
> > Fix potential null pointer dereference vulnerability for this.
> >
> > Signed-off-by: Baozeng Ding <sploving1@gmail.com>
>
> TIPC maintainers, please review.
>

Nak, nlattrs does not come from userspace in the compat layer, it just
translates to the legacy TLV thing.

//E
------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e

Re: [tipc-discussion] [PATCH] tipc: fix potential null pointer dereference in the nla_data function

[David Miller]

From: Erik Hugne <erik.hugne@gmail.com>
Date: Sat, 11 Jun 2016 09:46:32 +0200

> On Jun 11, 2016 08:24, "David Miller" <davem@davemloft.net> wrote:
>>
>> From: Baozeng Ding <sploving1@gmail.com>
>> Date: Fri, 10 Jun 2016 10:26:59 +0800
>>
>> > Before calling the nla_data function, make sure the argument is not
> null.
>> > Fix potential null pointer dereference vulnerability for this.
>> >
>> > Signed-off-by: Baozeng Ding <sploving1@gmail.com>
>>
>> TIPC maintainers, please review.
>>
> 
> Nak, nlattrs does not come from userspace in the compat layer, it just
> translates to the legacy TLV thing.
> 
> //E

This reply was done in a manner that the Message-ID got lost,
therefore patchwork cannot associate your reply with the original
patch posting and store it in the conversation attached to the patch.

Please don't do that in the future, thanks.