[DECNET] Endian bug fixes
Hi,
Here is a patch which fixes some endianess problems. Patrick: since you
have both big little endian machines at your disposal, can you test to
ensure this is ok? Thanks,
Steve.
From ed3de950e89f8b02302308a2bedd59123ff3b88e Mon Sep 17 00:00:00 2001
From: Steven Whitehouse [EMAIL PROTECTED]
Date: Mon, 6 Nov 2006 10:30:30 -0500
Subject: [PATCH] [DECNET] Endianess fixes
Here are some fixes to endianess problems spotted by Al Viro.
Cc: Al Viro [EMAIL PROTECTED]
Cc: Patrick Caulfield [EMAIL PROTECTED]
Signed-off-by: Steven Whitehouse [EMAIL PROTECTED]
---
net/decnet/af_decnet.c | 21 ++---
net/decnet/dn_rules.c |4 ++--
2 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index 3456cd3..37b4720 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -166,7 +166,7 @@ static struct hlist_head *dn_find_list(s
if (scp-addr.sdn_flags SDF_WILD)
return hlist_empty(dn_wild_sk) ? dn_wild_sk : NULL;
- return dn_sk_hash[scp-addrloc DN_SK_HASH_MASK];
+ return dn_sk_hash[dn_ntohs(scp-addrloc) DN_SK_HASH_MASK];
}
/*
@@ -180,7 +180,7 @@ static int check_port(__le16 port)
if (port == 0)
return -1;
- sk_for_each(sk, node, dn_sk_hash[port DN_SK_HASH_MASK]) {
+ sk_for_each(sk, node, dn_sk_hash[dn_ntohs(port) DN_SK_HASH_MASK]) {
struct dn_scp *scp = DN_SK(sk);
if (scp-addrloc == port)
return -1;
@@ -194,12 +194,12 @@ static unsigned short port_alloc(struct
static unsigned short port = 0x2000;
unsigned short i_port = port;
- while(check_port(++port) != 0) {
+ while(check_port(dn_htons(++port)) != 0) {
if (port == i_port)
return 0;
}
- scp-addrloc = port;
+ scp-addrloc = dn_htons(port);
return 1;
}
@@ -418,7 +418,7 @@ struct sock *dn_find_by_skb(struct sk_bu
struct dn_scp *scp;
read_lock(dn_hash_lock);
- sk_for_each(sk, node, dn_sk_hash[cb-dst_port DN_SK_HASH_MASK]) {
+ sk_for_each(sk, node, dn_sk_hash[dn_ntohs(cb-dst_port)
DN_SK_HASH_MASK]) {
scp = DN_SK(sk);
if (cb-src != dn_saddr2dn(scp-peer))
continue;
@@ -1016,13 +1016,12 @@ static void dn_access_copy(struct sk_buf
static void dn_user_copy(struct sk_buff *skb, struct optdata_dn *opt)
{
-unsigned char *ptr = skb-data;
-
-opt-opt_optl = *ptr++;
-opt-opt_status = 0;
-memcpy(opt-opt_data, ptr, opt-opt_optl);
-skb_pull(skb, dn_ntohs(opt-opt_optl) + 1);
+ unsigned char *ptr = skb-data;
+ opt-opt_optl = dn_htons((__u16)*ptr++);
+ opt-opt_status = 0;
+ memcpy(opt-opt_data, ptr, dn_ntohs(opt-opt_optl));
+ skb_pull(skb, dn_ntohs(opt-opt_optl) + 1);
}
static struct sk_buff *dn_wait_for_connect(struct sock *sk, long *timeo)
diff --git a/net/decnet/dn_rules.c b/net/decnet/dn_rules.c
index 3e0c882..590e0a7 100644
--- a/net/decnet/dn_rules.c
+++ b/net/decnet/dn_rules.c
@@ -124,8 +124,8 @@ static struct nla_policy dn_fib_rule_pol
static int dn_fib_rule_match(struct fib_rule *rule, struct flowi *fl, int
flags)
{
struct dn_fib_rule *r = (struct dn_fib_rule *)rule;
- u16 daddr = fl-fld_dst;
- u16 saddr = fl-fld_src;
+ __le16 daddr = fl-fld_dst;
+ __le16 saddr = fl-fld_src;
if (((saddr ^ r-src) r-srcmask) ||
((daddr ^ r-dst) r-dstmask))
--
1.4.1
Re: [DECNET] Endian bug fixes
On Mon, Nov 06, 2006 at 10:32:43AM +, Al Viro wrote: On Mon, Nov 06, 2006 at 10:31:02AM +, Steven Whitehouse wrote: + opt-opt_optl = dn_htons((__u16)*ptr++); Lose that cast; it's only confusing the things... + memcpy(opt-opt_data, ptr, dn_ntohs(opt-opt_optl)); + skb_pull(skb, dn_ntohs(opt-opt_optl) + 1); ... and I'd actually do u16 len = *ptr++; /* yes, it's 8bit on the wire */ opt-opt_optl = dn_htons(len); BUG_ON(len 16); /* we've checked the contents earlier */ memcpy(opt-opt_data, ptr, len); skb_pull(skb, len + 1); BTW, why the hell do we keep -opt_optl __le16 internally? If we ever pass it to userland, fine, but let's convert to __le16 *then*... - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [DECNET] Endian bug fixes
On Mon, Nov 06, 2006 at 10:31:02AM +, Steven Whitehouse wrote: + opt-opt_optl = dn_htons((__u16)*ptr++); Lose that cast; it's only confusing the things... + memcpy(opt-opt_data, ptr, dn_ntohs(opt-opt_optl)); + skb_pull(skb, dn_ntohs(opt-opt_optl) + 1); ... and I'd actually do u16 len = *ptr++; /* yes, it's 8bit on the wire */ opt-opt_optl = dn_htons(len); BUG_ON(len 16); /* we've checked the contents earlier */ memcpy(opt-opt_data, ptr, len); skb_pull(skb, len + 1); - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [DECNET] Endian bug fixes
Hi,
On Mon, 2006-11-06 at 10:32 +, Al Viro wrote:
On Mon, Nov 06, 2006 at 10:31:02AM +, Steven Whitehouse wrote:
+ opt-opt_optl = dn_htons((__u16)*ptr++);
Lose that cast; it's only confusing the things...
+ memcpy(opt-opt_data, ptr, dn_ntohs(opt-opt_optl));
+ skb_pull(skb, dn_ntohs(opt-opt_optl) + 1);
... and I'd actually do
u16 len = *ptr++; /* yes, it's 8bit on the wire */
opt-opt_optl = dn_htons(len);
BUG_ON(len 16); /* we've checked the contents earlier */
memcpy(opt-opt_data, ptr, len);
skb_pull(skb, len + 1);
Ok, and I've also made the same change in the other places too, so far
as its relevant in those cases. New patch attached,
Steve.
From a184f89a13fa292589f309057cc0775a8256a89e Mon Sep 17 00:00:00 2001
From: Steven Whitehouse [EMAIL PROTECTED]
Date: Mon, 6 Nov 2006 11:51:00 -0500
Subject: [DECNET] Endianess fixes (try #2)
Here are some fixes to endianess problems spotted by Al Viro.
Cc: Al Viro [EMAIL PROTECTED]
Cc: Patrick Caulfield [EMAIL PROTECTED]
Signed-off-by: Steven Whitehouse [EMAIL PROTECTED]
---
net/decnet/af_decnet.c | 25 +
net/decnet/dn_nsp_in.c |8
net/decnet/dn_nsp_out.c |2 +-
net/decnet/dn_rules.c |4 ++--
4 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index 3456cd3..21f20f2 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -166,7 +166,7 @@ static struct hlist_head *dn_find_list(s
if (scp-addr.sdn_flags SDF_WILD)
return hlist_empty(dn_wild_sk) ? dn_wild_sk : NULL;
- return dn_sk_hash[scp-addrloc DN_SK_HASH_MASK];
+ return dn_sk_hash[dn_ntohs(scp-addrloc) DN_SK_HASH_MASK];
}
/*
@@ -180,7 +180,7 @@ static int check_port(__le16 port)
if (port == 0)
return -1;
- sk_for_each(sk, node, dn_sk_hash[port DN_SK_HASH_MASK]) {
+ sk_for_each(sk, node, dn_sk_hash[dn_ntohs(port) DN_SK_HASH_MASK]) {
struct dn_scp *scp = DN_SK(sk);
if (scp-addrloc == port)
return -1;
@@ -194,12 +194,12 @@ static unsigned short port_alloc(struct
static unsigned short port = 0x2000;
unsigned short i_port = port;
- while(check_port(++port) != 0) {
+ while(check_port(dn_htons(++port)) != 0) {
if (port == i_port)
return 0;
}
- scp-addrloc = port;
+ scp-addrloc = dn_htons(port);
return 1;
}
@@ -418,7 +418,7 @@ struct sock *dn_find_by_skb(struct sk_bu
struct dn_scp *scp;
read_lock(dn_hash_lock);
- sk_for_each(sk, node, dn_sk_hash[cb-dst_port DN_SK_HASH_MASK]) {
+ sk_for_each(sk, node, dn_sk_hash[dn_ntohs(cb-dst_port)
DN_SK_HASH_MASK]) {
scp = DN_SK(sk);
if (cb-src != dn_saddr2dn(scp-peer))
continue;
@@ -1016,13 +1016,14 @@ static void dn_access_copy(struct sk_buf
static void dn_user_copy(struct sk_buff *skb, struct optdata_dn *opt)
{
-unsigned char *ptr = skb-data;
-
-opt-opt_optl = *ptr++;
-opt-opt_status = 0;
-memcpy(opt-opt_data, ptr, opt-opt_optl);
-skb_pull(skb, dn_ntohs(opt-opt_optl) + 1);
-
+ unsigned char *ptr = skb-data;
+ u16 len = *ptr++; /* yes, it's 8bit on the wire */
+
+ BUG_ON(len 16); /* we've checked the contents earlier */
+ opt-opt_optl = dn_htons(len);
+ opt-opt_status = 0;
+ memcpy(opt-opt_data, ptr, len);
+ skb_pull(skb, len + 1);
}
static struct sk_buff *dn_wait_for_connect(struct sock *sk, long *timeo)
diff --git a/net/decnet/dn_nsp_in.c b/net/decnet/dn_nsp_in.c
index 72ecc6e..7683d4f 100644
--- a/net/decnet/dn_nsp_in.c
+++ b/net/decnet/dn_nsp_in.c
@@ -360,9 +360,9 @@ static void dn_nsp_conn_conf(struct sock
scp-max_window = decnet_no_fc_max_cwnd;
if (skb-len 0) {
- unsigned char dlen = *skb-data;
+ u16 dlen = *skb-data;
if ((dlen = 16) (dlen = skb-len)) {
- scp-conndata_in.opt_optl =
dn_htons((__u16)dlen);
+ scp-conndata_in.opt_optl = dn_htons(dlen);
memcpy(scp-conndata_in.opt_data, skb-data +
1, dlen);
}
}
@@ -404,9 +404,9 @@ static void dn_nsp_disc_init(struct sock
memset(scp-discdata_in.opt_data, 0, 16);
if (skb-len 0) {
- unsigned char dlen = *skb-data;
+ u16 dlen = *skb-data;
if ((dlen = 16) (dlen = skb-len)) {
- scp-discdata_in.opt_optl = dn_htons((__u16)dlen);
+ scp-discdata_in.opt_optl = dn_htons(dlen);
memcpy(scp-discdata_in.opt_data, skb-data + 1, dlen);
}
}
Re: [DECNET] Endian bug fixes
Hi, On Mon, 2006-11-06 at 10:34 +, Al Viro wrote: On Mon, Nov 06, 2006 at 10:32:43AM +, Al Viro wrote: On Mon, Nov 06, 2006 at 10:31:02AM +, Steven Whitehouse wrote: + opt-opt_optl = dn_htons((__u16)*ptr++); Lose that cast; it's only confusing the things... + memcpy(opt-opt_data, ptr, dn_ntohs(opt-opt_optl)); + skb_pull(skb, dn_ntohs(opt-opt_optl) + 1); ... and I'd actually do u16 len = *ptr++; /* yes, it's 8bit on the wire */ opt-opt_optl = dn_htons(len); BUG_ON(len 16); /* we've checked the contents earlier */ memcpy(opt-opt_data, ptr, len); skb_pull(skb, len + 1); BTW, why the hell do we keep -opt_optl __le16 internally? If we ever pass it to userland, fine, but let's convert to __le16 *then*... Really the only thing that we do with this data is verify it and pass to userland. It does mean that getsockopt() is simpler for just being able to use copy_to_user() with a ptr len depending on which of the structures the user has requested rather than having to convert each field of each structure for example. I'm not sure its worth changing it now, for saving just one byte per socket in this case, Steve. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
