Message ID | 20110202190210.A9ED4F89F8@sepang.rtg.net |
---|---|
State | Accepted |
Delegated to: | Stefan Bader |
Headers | show |
On 02/02/2011 08:02 PM, 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@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@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> Acked-by: Stefan Bader <stefan.bader@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; > } >
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@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@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> > --- > 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@canonical.com>
applied and pushed