* [DECNET] Endian bug fixes @ 2006-11-06 10:31 Steven Whitehouse 2006-11-06 10:32 ` Al Viro 2006-11-07 22:59 ` David Miller 0 siblings, 2 replies; 6+ messages in thread From: Steven Whitehouse @ 2006-11-06 10:31 UTC (permalink / raw) To: Patrick Caulfield; +Cc: David Miller, netdev, Al Viro [-- Attachment #1: Type: text/plain, Size: 186 bytes --] 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. [-- Attachment #2: Patch to fix DECnet endianess bugs --] [-- Type: text/plain, Size: 3155 bytes --] >From ed3de950e89f8b02302308a2bedd59123ff3b88e Mon Sep 17 00:00:00 2001 From: Steven Whitehouse <swhiteho@redhat.com> 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 <viro@ftp.linux.org.uk> Cc: Patrick Caulfield <patrick@tykepenguin.com> Signed-off-by: Steven Whitehouse <swhiteho@redhat.com> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [DECNET] Endian bug fixes 2006-11-06 10:31 [DECNET] Endian bug fixes Steven Whitehouse @ 2006-11-06 10:32 ` Al Viro 2006-11-06 10:34 ` Al Viro 2006-11-06 11:50 ` Steven Whitehouse 2006-11-07 22:59 ` David Miller 1 sibling, 2 replies; 6+ messages in thread From: Al Viro @ 2006-11-06 10:32 UTC (permalink / raw) To: Steven Whitehouse; +Cc: Patrick Caulfield, David Miller, netdev On Mon, Nov 06, 2006 at 10:31:02AM +0000, 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); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [DECNET] Endian bug fixes 2006-11-06 10:32 ` Al Viro @ 2006-11-06 10:34 ` Al Viro 2006-11-06 11:50 ` Steven Whitehouse 2006-11-06 11:50 ` Steven Whitehouse 1 sibling, 1 reply; 6+ messages in thread From: Al Viro @ 2006-11-06 10:34 UTC (permalink / raw) To: Steven Whitehouse; +Cc: Patrick Caulfield, David Miller, netdev On Mon, Nov 06, 2006 at 10:32:43AM +0000, Al Viro wrote: > On Mon, Nov 06, 2006 at 10:31:02AM +0000, 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*... ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [DECNET] Endian bug fixes 2006-11-06 10:34 ` Al Viro @ 2006-11-06 11:50 ` Steven Whitehouse 0 siblings, 0 replies; 6+ messages in thread From: Steven Whitehouse @ 2006-11-06 11:50 UTC (permalink / raw) To: Al Viro; +Cc: Patrick Caulfield, David Miller, netdev Hi, On Mon, 2006-11-06 at 10:34 +0000, Al Viro wrote: > On Mon, Nov 06, 2006 at 10:32:43AM +0000, Al Viro wrote: > > On Mon, Nov 06, 2006 at 10:31:02AM +0000, 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [DECNET] Endian bug fixes 2006-11-06 10:32 ` Al Viro 2006-11-06 10:34 ` Al Viro @ 2006-11-06 11:50 ` Steven Whitehouse 1 sibling, 0 replies; 6+ messages in thread From: Steven Whitehouse @ 2006-11-06 11:50 UTC (permalink / raw) To: Al Viro; +Cc: Patrick Caulfield, David Miller, netdev [-- Attachment #1: Type: text/plain, Size: 720 bytes --] Hi, On Mon, 2006-11-06 at 10:32 +0000, Al Viro wrote: > On Mon, Nov 06, 2006 at 10:31:02AM +0000, 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. [-- Attachment #2: 0001-DECNET-Endianess-fixes-try-2.txt --] [-- Type: text/plain, Size: 4819 bytes --] >From a184f89a13fa292589f309057cc0775a8256a89e Mon Sep 17 00:00:00 2001 From: Steven Whitehouse <swhiteho@redhat.com> 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 <viro@ftp.linux.org.uk> Cc: Patrick Caulfield <patrick@tykepenguin.com> Signed-off-by: Steven Whitehouse <swhiteho@redhat.com> --- 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); } } diff --git a/net/decnet/dn_nsp_out.c b/net/decnet/dn_nsp_out.c index c2e21cd..b342e4e 100644 --- a/net/decnet/dn_nsp_out.c +++ b/net/decnet/dn_nsp_out.c @@ -526,7 +526,7 @@ void dn_send_conn_conf(struct sock *sk, struct nsp_conn_init_msg *msg; __u8 len = (__u8)dn_ntohs(scp->conndata_out.opt_optl); - if ((skb = dn_alloc_skb(sk, 50 + dn_ntohs(scp->conndata_out.opt_optl), gfp)) == NULL) + if ((skb = dn_alloc_skb(sk, 50 + len, gfp)) == NULL) return; msg = (struct nsp_conn_init_msg *)skb_put(skb, sizeof(*msg)); 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [DECNET] Endian bug fixes 2006-11-06 10:31 [DECNET] Endian bug fixes Steven Whitehouse 2006-11-06 10:32 ` Al Viro @ 2006-11-07 22:59 ` David Miller 1 sibling, 0 replies; 6+ messages in thread From: David Miller @ 2006-11-07 22:59 UTC (permalink / raw) To: swhiteho; +Cc: patrick, netdev, viro From: Steven Whitehouse <swhiteho@redhat.com> Date: Mon, 06 Nov 2006 10:31:02 +0000 > 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, Applied, thanks Steven and Al. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-11-07 22:59 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-11-06 10:31 [DECNET] Endian bug fixes Steven Whitehouse 2006-11-06 10:32 ` Al Viro 2006-11-06 10:34 ` Al Viro 2006-11-06 11:50 ` Steven Whitehouse 2006-11-06 11:50 ` Steven Whitehouse 2006-11-07 22:59 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).