Karmic 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:06:48 UTC 2011
On 02/02/2011 11:02 AM, Tim Gardner wrote:
> The following changes since commit 67180c643503bba1187f18c1fda2fd0725d5a957:
> Kulikov Vasiliy (1):
> net: tipc: fix information leak to userland, CVE-2010-3877
>
> are available in the git repository at:
>
> git://kernel.ubuntu.com/rtg/ubuntu-karmic.git CVE-2010-3880
>
> Nelson Elhage (1):
> inet_diag: Make sure we actually run the same bytecode we audited, CVE-2010-3880
>
> include/net/netlink.h | 2 +-
> net/ipv4/inet_diag.c | 27 ++++++++++++++++-----------
> 2 files changed, 17 insertions(+), 12 deletions(-)
>
> From 1a485553d24cd8021b0d0d86a97b9a5f036228a3 Mon Sep 17 00:00:00 2001
> From: Nelson Elhage<nelhage at ksplice.com>
> 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
>
> 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>
> ---
> 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 007bdb0..9cbc3fb 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -384,7 +384,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 a706a47..6fe360f 100644
> --- a/net/ipv4/inet_diag.c
> +++ b/net/ipv4/inet_diag.c
> @@ -489,9 +489,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;
> @@ -511,7 +513,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;
> }
>
> @@ -526,9 +528,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)
> @@ -547,7 +551,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;
> }
>
> @@ -617,7 +621,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;
> @@ -637,8 +641,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;
> }
> @@ -671,8 +676,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;
> }
>
Acked-by: Brad Figg <brad.figg at canonical.com>
--
Brad Figg brad.figg at canonical.com http://www.canonical.com
More information about the kernel-team
mailing list