Re: cls_u32: check unaligned data access

Previous thread: [PATCH] act_pedit: access skb->data safely by Changli Gao on Wednesday, June 2, 2010 - 7:55 am. (2 messages)

Next thread: [PATCH] net: mac8390 - Sort out memory/MMIO accesses and casts by Geert Uytterhoeven on Wednesday, June 2, 2010 - 10:36 am. (3 messages)
From: Changli Gao
Date: Wednesday, June 2, 2010 - 8:15 am

check unaligned data access

before accessing data, check if the corresponding address is aligned, and if
not, return -1.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/sched/cls_u32.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 4f52214..309d275 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -102,7 +102,8 @@ static int u32_classify(struct sk_buff *skb, struct tcf_proto *tp, struct tcf_re
 	} stack[TC_U32_MAXDEPTH];
 
 	struct tc_u_hnode *ht = (struct tc_u_hnode*)tp->root;
-	unsigned int off = skb_network_offset(skb);
+	unsigned int noff = skb_network_offset(skb);
+	unsigned int off = noff;
 	struct tc_u_knode *n;
 	int sdepth = 0;
 	int off2 = 0;
@@ -138,6 +139,8 @@ next_knode:
 			__be32 *data, _data;
 
 			toff = off + key->off + (off2 & key->offmask);
+			if ((toff - noff) % 4)
+				goto out;
 			data = skb_header_pointer(skb, toff, 4, &_data);
 			if (!data)
 				goto out;
@@ -188,6 +191,8 @@ check_terminal:
 		if (ht->divisor) {
 			__be32 *data, _data;
 
+			if ((off + n->sel.hoff - noff) % 4)
+				goto out;
 			data = skb_header_pointer(skb, off + n->sel.hoff, 4,
 						  &_data);
 			if (!data)
@@ -203,6 +208,8 @@ check_terminal:
 			if (n->sel.flags & TC_U32_VAROFFSET) {
 				__be16 *data, _data;
 
+				if ((off + n->sel.offoff - noff) % 2)
+					goto out;
 				data = skb_header_pointer(skb,
 							  off + n->sel.offoff,
 							  2, &_data);
--

From: David Miller
Date: Wednesday, June 2, 2010 - 8:18 am

From: Changli Gao <xiaosuo@gmail.com>

The user will find out when he gets warnings in his kernel log
messages on platforms where this matters.

And, if anything, silently just skipping over things is not
acceptable.  And imposing a 4-byte alignment could break
existing setups that actually work on x86 and powerpc which
are platforms that don't have alignment issues.

I basically tried to explain to you earlier that I wasn't going to
accept patches that try to deal with alignment in any way here in this
code, we explicitly and intentionally blindly dereference the data.

--

Previous thread: [PATCH] act_pedit: access skb->data safely by Changli Gao on Wednesday, June 2, 2010 - 7:55 am. (2 messages)

Next thread: [PATCH] net: mac8390 - Sort out memory/MMIO accesses and casts by Geert Uytterhoeven on Wednesday, June 2, 2010 - 10:36 am. (3 messages)