On Mon, May 18, 2020 at 03:50:05PM +0200, Patrick Wildt wrote:
> Hi,
>
> I was trying to tftpboot and had an issue with files of odd-length.
> As it turns out, I think the in_cksum() that's called for UDP payload
> cannot handle a payload length that's not aligned to 16 bytes.
>
> I don't know how in_cksum() is supposed to work exactly, but it looks
> like the first step is summing up all bytes. The code is using 16-
> byte blocks, apart from some oddbyte magic.
>
> First of all, why is there a while loop around code that already
> consumes the whole length? That can be done in a single step
> without the loop. Why does it continue of there's an "oddbyte"?
>
> If I simplify that whole construct, consuming in 16-bytes step
> until there's only one left, then summing that one, in_cksum()
> works for me.
>
> Can someone please help me have a look?
There are other versions of in_cksum in our tree.
Like: ./usr.sbin/ospfd/in_cksum.c
I'm surprised that the libsa code does no htons / ntohs conversions.
Also after looking at the ospfd/in_cksum.c code I wonder if the htons /
ntohs are actually reversed in that that code...
> Patrick
>
> diff --git a/sys/lib/libsa/in_cksum.c b/sys/lib/libsa/in_cksum.c
> index d3f2e6ac978..57ded38a7b7 100644
> --- a/sys/lib/libsa/in_cksum.c
> +++ b/sys/lib/libsa/in_cksum.c
> @@ -59,31 +59,24 @@
> int
> in_cksum(const void *p, int len)
> {
> - int sum = 0, oddbyte = 0, v = 0;
> const u_char *cp = p;
> + int sum = 0;
>
> /* we assume < 2^16 bytes being summed */
> - while (len > 0) {
> - if (oddbyte) {
> - sum += v + *cp++;
> - len--;
> - }
> + while (len > 1) {
> if (((long)cp & 1) == 0) {
> - while ((len -= 2) >= 0) {
> - sum += *(const u_short *)cp;
> - cp += 2;
> - }
> + sum += *(const u_short *)cp;
> + cp += 2;
> } else {
> - while ((len -= 2) >= 0) {
> - sum += *cp++ << 8;
> - sum += *cp++;
> - }
> + sum += *cp++ << 8;
> + sum += *cp++;
> }
> - if ((oddbyte = len & 1) != 0)
> - v = *cp << 8;
> + len -= 2;
> + }
> + if (len > 0) {
> + sum += *cp++;
> + len--;
> }
> - if (oddbyte)
> - sum += v;
> sum = (sum >> 16) + (sum & 0xffff); /* add in accumulated carries */
> sum += sum >> 16; /* add potential last carry */
> return (0xffff & ~sum);
>
--
:wq Claudio
