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

login
register
mail settings
Submitter Tim Gardner
Date Feb. 2, 2011, 6:58 p.m.
Message ID <20110202185818.1A7FBF89F8@sepang.rtg.net>
Download mbox
Permalink /patch/81514/
State Accepted
Delegated to: Stefan Bader
Headers show

Pull-request

git://kernel.ubuntu.com/rtg/ubuntu-maverick.git CVE-2010-3880

Comments

Tim Gardner - Feb. 2, 2011, 6:58 p.m.
The following changes since commit 8c8177a53331b2c0a49df5ca4b44e4700ad550e8:
  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-maverick.git CVE-2010-3880

Nelson Elhage (1):
      inet_diag: Make sure we actually run the same bytecode we audited, CVE-2010-3880

 net/ipv4/inet_diag.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

From fc7762049ad234ce6e790a3b68528928521af63e 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>
(cherry picked from commit 22e76c849d505d87c5ecf3d3e6742a65f0ff4860)

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 net/ipv4/inet_diag.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)
Stefan Bader - Feb. 7, 2011, 8:31 a.m.
On 02/02/2011 07:58 PM, Tim Gardner wrote:
> The following changes since commit 8c8177a53331b2c0a49df5ca4b44e4700ad550e8:
>   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-maverick.git CVE-2010-3880
> 
> Nelson Elhage (1):
>       inet_diag: Make sure we actually run the same bytecode we audited, CVE-2010-3880
> 
>  net/ipv4/inet_diag.c |   27 ++++++++++++++++-----------
>  1 files changed, 16 insertions(+), 11 deletions(-)
> 
> From fc7762049ad234ce6e790a3b68528928521af63e 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>
> (cherry picked 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 |   27 ++++++++++++++++-----------
>  1 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
> index e5fa2dd..7403b9b 100644
> --- a/net/ipv4/inet_diag.c
> +++ b/net/ipv4/inet_diag.c
> @@ -490,9 +490,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;
> @@ -512,7 +514,7 @@ static int inet_csk_diag_dump(struct sock *sk,
>  		entry.dport = ntohs(inet->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;
>  	}
>  
> @@ -527,9 +529,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)
> @@ -548,7 +552,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;
>  	}
>  
> @@ -618,7 +622,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;
> @@ -638,8 +642,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->inet_num;
>  		entry.userlocks = sk->sk_userlocks;
>  	}
> @@ -672,8 +677,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;
>  			}
>
Brad Figg - Feb. 9, 2011, 8:08 p.m.
On 02/02/2011 10:58 AM, Tim Gardner wrote:
> The following changes since commit 8c8177a53331b2c0a49df5ca4b44e4700ad550e8:
>    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-maverick.git CVE-2010-3880
>
> Nelson Elhage (1):
>        inet_diag: Make sure we actually run the same bytecode we audited, CVE-2010-3880
>
>   net/ipv4/inet_diag.c |   27 ++++++++++++++++-----------
>   1 files changed, 16 insertions(+), 11 deletions(-)
>
>  From fc7762049ad234ce6e790a3b68528928521af63e 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>
> (cherry picked from commit 22e76c849d505d87c5ecf3d3e6742a65f0ff4860)
>
> Signed-off-by: Tim Gardner<tim.gardner@canonical.com>
> ---
>   net/ipv4/inet_diag.c |   27 ++++++++++++++++-----------
>   1 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
> index e5fa2dd..7403b9b 100644
> --- a/net/ipv4/inet_diag.c
> +++ b/net/ipv4/inet_diag.c
> @@ -490,9 +490,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;
> @@ -512,7 +514,7 @@ static int inet_csk_diag_dump(struct sock *sk,
>   		entry.dport = ntohs(inet->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;
>   	}
>
> @@ -527,9 +529,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)
> @@ -548,7 +552,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;
>   	}
>
> @@ -618,7 +622,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;
> @@ -638,8 +642,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->inet_num;
>   		entry.userlocks = sk->sk_userlocks;
>   	}
> @@ -672,8 +677,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>