diff mbox

[hardy,lucid/fsl-imx51,maverick,maverick/ti-omap4,natty,natty/ti-omap4,CVE,1/1] inet_diag: fix inet_diag_bc_audit()

Message ID 1316015475-17287-2-git-send-email-apw@canonical.com
State New
Headers show

Commit Message

Andy Whitcroft Sept. 14, 2011, 3:51 p.m. UTC 
From: Eric Dumazet <eric.dumazet@gmail.com>

A malicious user or buggy application can inject code and trigger an
infinite loop in inet_diag_bc_audit()

Also make sure each instruction is aligned on 4 bytes boundary, to avoid
unaligned accesses.

Reported-by: Dan Rosenberg <drosenberg@vsecurity.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

(cherry picked from commit eeb1497277d6b1a0a34ed36b97e18f2bd7d6de0d)
CVE-2011-2213
BugLink: http://bugs.launchpad.net/bugs/838421
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
 net/ipv4/inet_diag.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

Comments

Seth Forshee Sept. 14, 2011, 4:36 p.m. UTC | #1 
On Wed, Sep 14, 2011 at 04:51:15PM +0100, Andy Whitcroft wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> 
> A malicious user or buggy application can inject code and trigger an
> infinite loop in inet_diag_bc_audit()
> 
> Also make sure each instruction is aligned on 4 bytes boundary, to avoid
> unaligned accesses.
> 
> Reported-by: Dan Rosenberg <drosenberg@vsecurity.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> (cherry picked from commit eeb1497277d6b1a0a34ed36b97e18f2bd7d6de0d)
> CVE-2011-2213
> BugLink: http://bugs.launchpad.net/bugs/838421
> Signed-off-by: Andy Whitcroft <apw@canonical.com>

It's a clean cherry-pick, and looks like it should fix the problem as
described. I don't know about the correctness of the alignment
enforcement, but since it appears to have been upstream for a while now
I'll assume it's okay.

Acked-by: Seth Forshee <seth.forshee@canonical.com>
diff mbox

Patch

diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index bf49652..c60ae4e 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -442,7 +442,7 @@  static int valid_cc(const void *bc, int len, int cc)
 			return 0;
 		if (cc == len)
 			return 1;
-		if (op->yes < 4)
+		if (op->yes < 4 || op->yes & 3)
 			return 0;
 		len -= op->yes;
 		bc  += op->yes;
@@ -452,11 +452,11 @@  static int valid_cc(const void *bc, int len, int cc)
 
 static int inet_diag_bc_audit(const void *bytecode, int bytecode_len)
 {
-	const unsigned char *bc = bytecode;
+	const void *bc = bytecode;
 	int  len = bytecode_len;
 
 	while (len > 0) {
-		struct inet_diag_bc_op *op = (struct inet_diag_bc_op *)bc;
+		const struct inet_diag_bc_op *op = bc;
 
 //printk("BC: %d %d %d {%d} / %d\n", op->code, op->yes, op->no, op[1].no, len);
 		switch (op->code) {
@@ -467,22 +467,20 @@  static int inet_diag_bc_audit(const void *bytecode, int bytecode_len)
 		case INET_DIAG_BC_S_LE:
 		case INET_DIAG_BC_D_GE:
 		case INET_DIAG_BC_D_LE:
-			if (op->yes < 4 || op->yes > len + 4)
-				return -EINVAL;
 		case INET_DIAG_BC_JMP:
-			if (op->no < 4 || op->no > len + 4)
+			if (op->no < 4 || op->no > len + 4 || op->no & 3)
 				return -EINVAL;
 			if (op->no < len &&
 			    !valid_cc(bytecode, bytecode_len, len - op->no))
 				return -EINVAL;
 			break;
 		case INET_DIAG_BC_NOP:
-			if (op->yes < 4 || op->yes > len + 4)
-				return -EINVAL;
 			break;
 		default:
 			return -EINVAL;
 		}
+		if (op->yes < 4 || op->yes > len + 4 || op->yes & 3)
+			return -EINVAL;
 		bc  += op->yes;
 		len -= op->yes;
 	}