| From stable-bounces@linux.kernel.org Tue Nov 13 00:10:11 2007 |
| From: Radu Rendec <radu.rendec@ines.ro> |
| Date: Tue, 13 Nov 2007 00:09:56 -0800 (PST) |
| Subject: Fix endianness bug in U32 classifier. |
| To: stable@kernel.org |
| Cc: bunk@kernel.org |
| Message-ID: <20071113.000956.33032860.davem@davemloft.net> |
| |
| From: Radu Rendec <radu.rendec@ines.ro> |
| |
| changeset 543821c6f5dea5221426eaf1eac98b100249c7ac in mainline. |
| |
| [PKT_SCHED] CLS_U32: Fix endianness problem with u32 classifier hash masks. |
| |
| While trying to implement u32 hashes in my shaping machine I ran into |
| a possible bug in the u32 hash/bucket computing algorithm |
| (net/sched/cls_u32.c). |
| |
| The problem occurs only with hash masks that extend over the octet |
| boundary, on little endian machines (where htonl() actually does |
| something). |
| |
| Let's say that I would like to use 0x3fc0 as the hash mask. This means |
| 8 contiguous "1" bits starting at b6. With such a mask, the expected |
| (and logical) behavior is to hash any address in, for instance, |
| 192.168.0.0/26 in bucket 0, then any address in 192.168.0.64/26 in |
| bucket 1, then 192.168.0.128/26 in bucket 2 and so on. |
| |
| This is exactly what would happen on a big endian machine, but on |
| little endian machines, what would actually happen with current |
| implementation is 0x3fc0 being reversed (into 0xc03f0000) by htonl() |
| in the userspace tool and then applied to 192.168.x.x in the u32 |
| classifier. When shifting right by 16 bits (rank of first "1" bit in |
| the reversed mask) and applying the divisor mask (0xff for divisor |
| 256), what would actually remain is 0x3f applied on the "168" octet of |
| the address. |
| |
| One could say is this can be easily worked around by taking endianness |
| into account in userspace and supplying an appropriate mask (0xfc03) |
| that would be turned into contiguous "1" bits when reversed |
| (0x03fc0000). But the actual problem is the network address (inside |
| the packet) not being converted to host order, but used as a |
| host-order value when computing the bucket. |
| |
| Let's say the network address is written as n31 n30 ... n0, with n0 |
| being the least significant bit. When used directly (without any |
| conversion) on a little endian machine, it becomes n7 ... n0 n8 ..n15 |
| etc in the machine's registers. Thus bits n7 and n8 would no longer be |
| adjacent and 192.168.64.0/26 and 192.168.128.0/26 would no longer be |
| consecutive. |
| |
| The fix is to apply ntohl() on the hmask before computing fshift, |
| and in u32_hash_fold() convert the packet data to host order before |
| shifting down by fshift. |
| |
| With helpful feedback from Jamal Hadi Salim and Jarek Poplawski. |
| |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> |
| |
| --- |
| net/sched/cls_u32.c | 4 ++-- |
| 1 file changed, 2 insertions(+), 2 deletions(-) |
| |
| --- a/net/sched/cls_u32.c |
| +++ b/net/sched/cls_u32.c |
| @@ -91,7 +91,7 @@ static struct tc_u_common *u32_list; |
| |
| static __inline__ unsigned u32_hash_fold(u32 key, struct tc_u32_sel *sel, u8 fshift) |
| { |
| - unsigned h = (key & sel->hmask)>>fshift; |
| + unsigned h = ntohl(key & sel->hmask)>>fshift; |
| |
| return h; |
| } |
| @@ -615,7 +615,7 @@ static int u32_change(struct tcf_proto * |
| n->handle = handle; |
| { |
| u8 i = 0; |
| - u32 mask = s->hmask; |
| + u32 mask = ntohl(s->hmask); |
| if (mask) { |
| while (!(mask & 1)) { |
| i++; |