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

[Stefan Bader]

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 at 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 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>
Acked-by: Stefan Bader <stefan.bader at 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;
>  			}
>