Message ID | 20110202190454.7F466F89F8@sepang.rtg.net |
---|---|
State | Accepted |
Delegated to: | Stefan Bader |
Headers | show |
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@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@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> > --- > 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); > 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; > } >
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@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@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> >> --- >> 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; >> } >> > >
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@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@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> >>> --- >>> 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