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

Brad Figg brad.figg at canonical.com
Wed Feb 9 20:40:47 UTC 2011


On 02/07/2011 07:01 AM, Stefan Bader wrote:
> On 02/02/2011 08:04 PM, Tim Gardner wrote:
>> The following changes since commit 2677c2506a252a7a8f322434ce3e3ae7acffdd39:
>>    Thomas Gleixner (1):
>>          x86: replace LOCK_PREFIX in futex.h, CVE-2010-3086
>>
>> are available in the git repository at:
>>
>>    git://kernel.ubuntu.com/rtg/ubuntu-dapper.git CVE-2010-3880
>>
>> Tim Gardner (1):
>>        inet_diag: Make sure we actually run the same bytecode we audited, CVE-2010-3880
>>
>>   net/ipv4/inet_diag.c |   17 +++++++++--------
>>   1 files changed, 9 insertions(+), 8 deletions(-)
>>
>>  From e89143e20d2e91b55d7b1f78b57304dae7648160 Mon Sep 17 00:00:00 2001
>> From: Tim Gardner<tim.gardner at 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 at ksplice.com>
>> Signed-off-by: Thomas Graf<tgraf at infradead.org>
>> Signed-off-by: David S. Miller<davem at davemloft.net>
>> (back ported from commit 22e76c849d505d87c5ecf3d3e6742a65f0ff4860)
>>
>> Signed-off-by: Tim Gardner<tim.gardner at canonical.com>
> Acked-by: Stefan Bader<stefan.bader at 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..00c85b1 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 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);


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


-- 
Brad Figg brad.figg at canonical.com http://www.canonical.com



More information about the kernel-team mailing list