diff mbox

Dapper CVE-2010-3880, inet_diag: Make sure we actually run the same bytecode we audited

Message ID 4D530162.7020304@canonical.com
State Accepted
Headers show

Commit Message

Tim Gardner Feb. 9, 2011, 9:04 p.m. UTC 
> On 02/09/2011 01:40 PM, Brad Figg wrote:
>>>> struct inet_diag_entry entry;
>>>> - struct rtattr *bc = (struct rtattr *)(r + 1);
>>>> + struct rtattr *bc = nlmsg_find_attr(cb->nlh, sizeof(*r),
>>>> + INET_DIAG_REQ_BYTECODE);
>>
>> In the other patches this was changed to:
>> const struct nlattr *bc = nlmsg_find_attr(cb->nlh,
>> sizeof(*r),
>> INET_DIAG_REQ_BYTECODE);
>>
>>
>
> I dropped 'const' because the prototype for nlmsg_find_attr() doesn't
> call for it in 2.6.15.
>

OK, I've corrected the type change from rtattr to nlattr.

rtg

Comments

Brad Figg Feb. 9, 2011, 9:05 p.m. UTC | #1 
On 02/09/2011 01:04 PM, Tim Gardner wrote:
>> On 02/09/2011 01:40 PM, Brad Figg wrote:
>>>>> struct inet_diag_entry entry;
>>>>> - struct rtattr *bc = (struct rtattr *)(r + 1);
>>>>> + struct rtattr *bc = nlmsg_find_attr(cb->nlh, sizeof(*r),
>>>>> + INET_DIAG_REQ_BYTECODE);
>>>
>>> In the other patches this was changed to:
>>> const struct nlattr *bc = nlmsg_find_attr(cb->nlh,
>>> sizeof(*r),
>>> INET_DIAG_REQ_BYTECODE);
>>>
>>>
>>
>> I dropped 'const' because the prototype for nlmsg_find_attr() doesn't
>> call for it in 2.6.15.
>>
>
> OK, I've corrected the type change from rtattr to nlattr.
>
> rtg

Acked-by: Brad Figg <brad.figg@canonical.com>
Tim Gardner Feb. 9, 2011, 9:12 p.m. UTC | #2 
applied and pushed
diff mbox

Patch

From 91b43948e129b6541e12dcb8b3e92ddd976f8753 Mon Sep 17 00:00:00 2001
From: Tim Gardner <tim.gardner@canonical.com>
Date: Wed, 2 Feb 2011 11:10:04 -0700
Subject: [PATCH] inet_diag: Make sure we actually run the same bytecode we audited, CVE-2010-3880

BugLink: http://bugs.launchpad.net/bugs/711865

CVE-2010-3880

We were using nlmsg_find_attr() to look up the bytecode by attribute when
auditing, but then just using the first attribute when actually running
bytecode. So, if we received a message with two attribute elements, where only
the second had type INET_DIAG_REQ_BYTECODE, we would validate and run different
bytecode strings.

Fix this by consistently using nlmsg_find_attr everywhere.

Signed-off-by: Nelson Elhage <nelhage@ksplice.com>
Signed-off-by: Thomas Graf <tgraf@infradead.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
(back ported from commit 22e76c849d505d87c5ecf3d3e6742a65f0ff4860)

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 net/ipv4/inet_diag.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 39061ed..8295872 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -28,12 +28,12 @@ 
 #include <net/inet_hashtables.h>
 #include <net/inet_timewait_sock.h>
 #include <net/inet6_hashtables.h>
+#include <net/netlink.h>
 
 #include <linux/inet.h>
 #include <linux/stddef.h>
 
 #include <linux/inet_diag.h>
-
 static const struct inet_diag_handler **inet_diag_table;
 
 struct inet_diag_entry {
@@ -418,9 +418,10 @@  static int inet_diag_dump_sock(struct sk_buff *skb, struct sock *sk,
 {
 	struct inet_diag_req *r = NLMSG_DATA(cb->nlh);
 
-	if (cb->nlh->nlmsg_len > 4 + NLMSG_SPACE(sizeof(*r))) {
+	if (nlmsg_attrlen(cb->nlh, sizeof(*r))) {
 		struct inet_diag_entry entry;
-		struct rtattr *bc = (struct rtattr *)(r + 1);
+		struct nlattr *bc = nlmsg_find_attr(cb->nlh, sizeof(*r),
+						INET_DIAG_REQ_BYTECODE);
 		struct inet_sock *inet = inet_sk(sk);
 
 		entry.family = sk->sk_family;
@@ -440,7 +441,7 @@  static int inet_diag_dump_sock(struct sk_buff *skb, struct sock *sk,
 		entry.dport = ntohs(inet->dport);
 		entry.userlocks = sk->sk_userlocks;
 
-		if (!inet_diag_bc_run(RTA_DATA(bc), RTA_PAYLOAD(bc), &entry))
+		if (!inet_diag_bc_run(nla_data(bc), nla_len(bc), &entry))
 			return 0;
 	}
 
@@ -530,8 +531,8 @@  static int inet_diag_dump_reqs(struct sk_buff *skb, struct sock *sk,
 	if (!lopt || !lopt->qlen)
 		goto out;
 
-	if (cb->nlh->nlmsg_len > 4 + NLMSG_SPACE(sizeof(*r))) {
-		bc = (struct rtattr *)(r + 1);
+	if (nlmsg_attrlen(cb->nlh, sizeof(*r))) {
+		bc = nlmsg_find_attr(cb->nlh, sizeof(*r), INET_DIAG_REQ_BYTECODE);
 		entry.sport = inet->num;
 		entry.userlocks = sk->sk_userlocks;
 	}
@@ -564,8 +565,8 @@  static int inet_diag_dump_reqs(struct sk_buff *skb, struct sock *sk,
 					&ireq->rmt_addr;
 				entry.dport = ntohs(ireq->rmt_port);
 
-				if (!inet_diag_bc_run(RTA_DATA(bc),
-						    RTA_PAYLOAD(bc), &entry))
+				if (!inet_diag_bc_run(nla_data(bc),
+						    nla_len(bc), &entry))
 					continue;
 			}
 
-- 
1.7.0.4