From patchwork Wed Feb 9 14:15:50 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Gardner X-Patchwork-Id: 82466 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id 6214EB70E3 for ; Thu, 10 Feb 2011 01:16:03 +1100 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1PnApn-0008M3-Qy; Wed, 09 Feb 2011 14:15:55 +0000 Received: from mail.tpi.com ([70.99.223.143]) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1PnApk-0008LU-T9 for kernel-team@lists.ubuntu.com; Wed, 09 Feb 2011 14:15:53 +0000 Received: from [10.0.2.5] (unknown [10.0.2.5]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mail.tpi.com (Postfix) with ESMTP id 8C8C8297D33 for ; Wed, 9 Feb 2011 06:15:43 -0800 (PST) Message-ID: <4D52A196.5040009@canonical.com> Date: Wed, 09 Feb 2011 07:15:50 -0700 From: Tim Gardner User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101208 Thunderbird/3.1.7 MIME-Version: 1.0 To: kernel-team@lists.ubuntu.com Subject: Re: Hardy CVE-2010-3880, inet_diag: Make sure we actually run the same bytecode we audited (V2) References: <20110202190335.46C19F89F8@sepang.rtg.net> <4D49AC68.9030301@canonical.com> <4D49C1DE.3070206@canonical.com> <4D4FB1CB.50602@canonical.com> <20110207091511.GN16804@shadowen.org> <4D5003CF.4040004@canonical.com> <4D529FC3.1010305@canonical.com> In-Reply-To: <4D529FC3.1010305@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.13 Precedence: list Reply-To: tim.gardner@canonical.com List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kernel-team-bounces@lists.ubuntu.com Errors-To: kernel-team-bounces@lists.ubuntu.com On 02/09/2011 07:08 AM, Tim Gardner wrote: > On 02/07/2011 07:38 AM, Tim Gardner wrote: >> On 02/07/2011 02:15 AM, Andy Whitcroft wrote: >>> On Mon, Feb 07, 2011 at 09:48:11AM +0100, Stefan Bader wrote: >>>> On 02/02/2011 09:43 PM, Tim Gardner wrote: >>>>> On 02/02/2011 12:11 PM, Tim Gardner wrote: >>>>>> Postpone this one for a bit. The custom binary openvz flavour is >>>>>> failing. >>>>>> >>>>>> rtg >>>>> >>>>> Added the openvz cleanup patch. >>>>> >>>>> rtg >>>>> >>>> Hm, this one looks odd. The hunk in the openvz patch seems to add the >>>> "sll->sll_plttype = 0" and that does not seem to be touched by this >>>> cve. >>> >>> That sounds familiar. I think that that comes from the CVE below: >>> >>> commit 2c14b0a36cebda5a1fc2c69f2dd01eb73365aa65 >>> Author: Andy Whitcroft >>> Date: Tue Feb 1 14:26:24 2011 +0000 >>> >>> net: packet: fix information leak to userland, CVE-2010-3876 >>> >>> I suspect that means this extra line should have come in earlier, not >>> sure if it is worth splitting out now, but in a perfect world I suspect >>> it should have been. My fault by the looks of it. >>> >>> -apw >>> >> >> Yeah, I think it's still OK. It just moves a hunk of a patch around, and >> the context lines go along for the ride. The custom binary patch >> mechanism is a real pain. >> >> rtg > > I fixed 'net: packet: fix information leak to userland, CVE-2010-3876' > for openvz and pushed +master-next, so you'll need to re-fetch. > > Now, back to the original Hardy CVE patch.... OK, how about this. rtg From 73a135d8bd6a932ee0fa1c4301eb007fea4c51b6 Mon Sep 17 00:00:00 2001 From: Nelson Elhage Date: Wed, 3 Nov 2010 16:35:41 +0000 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 openvz: Fixup after 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 Signed-off-by: Thomas Graf Signed-off-by: David S. Miller (back ported from commit 22e76c849d505d87c5ecf3d3e6742a65f0ff4860) Signed-off-by: Tim Gardner --- include/net/netlink.h | 2 +- net/ipv4/inet_diag.c | 27 ++++++++++++++++----------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/include/net/netlink.h b/include/net/netlink.h index 9298218..97caec7 100644 --- a/include/net/netlink.h +++ b/include/net/netlink.h @@ -383,7 +383,7 @@ static inline int nlmsg_parse(struct nlmsghdr *nlh, int hdrlen, * * Returns the first attribute which matches the specified type. */ -static inline struct nlattr *nlmsg_find_attr(struct nlmsghdr *nlh, +static inline struct nlattr *nlmsg_find_attr(const struct nlmsghdr *nlh, int hdrlen, int attrtype) { return nla_find(nlmsg_attrdata(nlh, hdrlen), diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c index 6d2979c..bf49652 100644 --- a/net/ipv4/inet_diag.c +++ b/net/ipv4/inet_diag.c @@ -495,9 +495,11 @@ static int inet_csk_diag_dump(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); + 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; @@ -517,7 +519,7 @@ static int inet_csk_diag_dump(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; } @@ -532,9 +534,11 @@ static int inet_twsk_diag_dump(struct inet_timewait_sock *tw, { 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); + const struct nlattr *bc = nlmsg_find_attr(cb->nlh, + sizeof(*r), + INET_DIAG_REQ_BYTECODE); entry.family = tw->tw_family; #if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE) @@ -553,7 +557,7 @@ static int inet_twsk_diag_dump(struct inet_timewait_sock *tw, entry.dport = ntohs(tw->tw_dport); entry.userlocks = 0; - 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; } @@ -623,7 +627,7 @@ static int inet_diag_dump_reqs(struct sk_buff *skb, struct sock *sk, struct inet_diag_req *r = NLMSG_DATA(cb->nlh); struct inet_connection_sock *icsk = inet_csk(sk); struct listen_sock *lopt; - struct rtattr *bc = NULL; + const struct nlattr *bc = NULL; struct inet_sock *inet = inet_sk(sk); int j, s_j; int reqnum, s_reqnum; @@ -643,8 +647,9 @@ 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; } @@ -677,8 +682,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