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

[Tim Gardner]

On 02/09/2011 01:40 PM, Brad Figg wrote:
> 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);
>
>

I dropped 'const' because the prototype for nlmsg_find_attr() doesn't 
call for it in 2.6.15.

rtg
-- 
Tim Gardner tim.gardner at canonical.com