Bug#23656: Reversed Appletalk Header Bytes (fwd)

Herbert Xu (herbert@gondor.apana.org.au)
Thu, 18 Jun 1998 20:07:02 +1000 (EST)


The code segment in question hasn't changed in 2.0.34 and a quick look at
2.0.106 seems to indicate that it's still the same there.

----- Forwarded message from Bryan Batten -----

Date: Wed, 17 Jun 1998 17:22:17 -0400
From: Bryan Batten <BryanBatten@compuserve.com>
To: Bugs in Debian <submit@bugs.debian.org>
Message-ID: <199806171722_MC2-408D-6632@compuserve.com>

Package: kernel-source-2.0.33
Version: 2.0.33-3

While trying unsuccessfully to share a LaserWriter Select 300 hooked up to
the printer port on a MacIIcx with a Pentium90 running Linux, I wrote a tap
to monitor the network traffic over the ethernet connection betwen the two
boxes using the SOCK_PACKET capability. e.g.

int s=socket(AF_INET,SOCK_PACKET,htons(ETH_P_ALL));

to run on my Linux box, and found that AARP function code values and DDP
length values were nonsensical under certain circumstances.

AARP packets that originated on the Linux box had an OK function field, but
packets that came from the Mac showed a nonsensical function field. DDP
packets that originated on the Linux box had an OK length field, but
packets that came from the Mac showed a nonsensical length field. I
eventually traced both problems to source files aarp.c and ddp.c in the
directory /usr/src/linux/net/appletalk. The section "Diff Output" shows the
changes made to both files to resolve these problems.

AARP Function Code
==================

Inspection of the code in aarp.c showed that the function aarp_rcv()
reverses the function code bytes in incoming packet data in order to
process them in host byte order. This has the result that my tap sees the
bytes in host byte order instead of network byte order as it was expecting.
The tap picks up locally originated packets OK because those function codes
are generated - and left - in network byte order.

By changing the logic of the function aarp_rcv() in aarp.c to leave the
protocol header of received packets untouched, and work with a temporary
copy instead, I was able to get consistent representations in network byte
order of the function code values - as observed by my tap - for both
locally and remotely originated packets.

DDP Length Field
================

Inspection of the code in ddp.c showed that the function atalk_rcv()
reverses the DDP protocol length field bytes in incoming packet data in
order to process them in host byte order. When routing, these bytes are
again reversed before the packet is passed on to another machine, so there
is no problem there. Remotely originated packets for which the local host
is the target however, are observed by my tap with the length field in host
byte order. However, locally originated broadcast packets are observed by
my tap with the length field still in network byte order.

This has the effect that a tap (mine, in this case) sees inconsistent data
length formats between locally originated packets and remotely originated
packets. Beyond observing that the inconsistency does exist, I haven't been
able to identify what software timing subtlety explains why it exists.

Modifications in ddp.c were made to two functions: atalk_rcv() was modified
to work with a temporary copy of the length field only, leaving the length
field in the packet untouched when the destination is the local host, but
continuing to update the hop count in packets destined for another machine.
atalk_recvmsg() was modified to expect network byte order in the length
field.

The result of the changes to ddp.c was that my tap now sees consistent
network byte order representations of the length field of DDP packets for
both locally originated and remotely originated packets.

Testing
=======

My environment consists of a MacIIcx running System 7.5.5 connected via
Ethernet to a Pentium 90 running Linux 2.0.33. I am able to aecho
successfully from the Linux machine to the Apple machine. Also, Apple file
sharing of Linux files on my Mac still works OK for all functions I can
think of that worked before.

My tap now captures all data consistently in network byte order.

Still can't share my printer, though.

Diff Output
===========

Here is the "diff" output for the current version (aarp.c.sv) and my
modified version (aarp.c):

============================================================
bash(root)# diff -w aarp.c.sv aarp.c >/tmp/bryan/diff.txt
646a647
> unsigned short function;
669c670
< ea->function=ntohs(ea->function);

---
>       function=ntohs(ea->function);
675c676
<       if(ea->function<AARP_REQUEST || ea->function > AARP_PROBE ||
ea->hw_len != ETH_ALEN || ea->pa_len != AARP_PA_ALEN ||
---
>       if(function<AARP_REQUEST || function > AARP_PROBE || ea->hw_len !=
ETH_ALEN || ea->pa_len != AARP_PA_ALEN ||
727c728
<       switch(ea->function)
---
>       switch(function)
============================================================

Here is the "diff" output for the current version (ddp.c.sv) and my modified version (ddp.c):

============================================================ bash(root)# diff -w ddp.c.sv ddp.c >>/tmp/bryan/diff.txt 1413a1414,1427 > union { > unsigned short uslen; > struct { > # ifdef __LITTLE_ENDIAN_BITFIELD > __u16 len:10, hops:4, pad:2; > # else > __u16 pad:2, hops:4, len:10; > # endif > } b; > } ddl; > #define ddl_hlen ddl.uslen > #define ddl_len ddl.b.len > #define ddl_hops ddl.b.hops > #define ddl_pad ddl.b.pad 1424,1426c1438,1439 < * Fix up the length field [Ok this is horrible but otherwise < * I end up with unions of bit fields and messy bit field order < * compiler/endian dependencies..]

---
>        *      Use a temporary value and work this field in host byte
order.
>        *      This is so any taps always will see network byte order.
1429c1442
<       *((__u16 *)ddp)=ntohs(*((__u16 *)ddp));
---
>       ddl_hlen=ntohs(*((__u16 *)ddp));
1437c1450
<       skb_trim(skb,min(skb->len,ddp->deh_len));
---
>       skb_trim(skb,min(skb->len,ddl_len));
1456c1469
<       if(ddp->deh_sum && atalk_checksum(ddp, ddp->deh_len)!=
ddp->deh_sum)
---
>       if(ddp->deh_sum && atalk_checksum(ddp, ddl_len)!= ddp->deh_sum)
1510c1523
<               if(rt==NULL || ddp->deh_hops==15)
---
>               if(rt==NULL || ddl_hops==15)
1515c1528
<               ddp->deh_hops++;
---
>               ddl_hops++;
1530c1543
<                       ddp_dl->header_length + ddp->deh_len));
---
>                       ddp_dl->header_length + ddl_len));
1532c1545
<               *((__u16 *)ddp)=ntohs(*((__u16 *)ddp));         /* Mend the
byte order */
---
>               *((__u16 *)ddp)=htons(ddl_hlen);        /* Update the hop
count */
1796,1798c1809
<        *      Fix up the length field [Ok this is horrible but otherwise
<        *      I end up with unions of bit fields and messy bit field
order
<        *      compiler/endian dependencies..
---
>        *      Convert the length field to network byte order.
1800c1811
<       *((__u16 *)ddp)=ntohs(*((__u16 *)ddp));
---
>       *((__u16 *)ddp)=htons(*((__u16 *)ddp));
1894a1906,1907
> /* Save the length bits after we have host byte order. */
> #define       DEH_LENM        0x03ff
1901c1914
<       int copied = 0;
---
>       int copied;
1915a1929
>       copied=ntohs(*((__u16 *)ddp)) & DEH_LENM;
1918d1931
<               copied=ddp->deh_len;
1925c1938
<               copied=ddp->deh_len - sizeof(*ddp);
---
>               copied -= sizeof(*ddp);
============================================================

Thanks,

Bryan

----- End of forwarded message from Bryan Batten -----

-- 
Debian GNU/Linux 1.3 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu