On Mon, May 18, 2020 at 05:50:28PM +0200, Claudio Jeker wrote:
> 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...
I copied ospfd's file, re-added the header includes, re-added const to
the pointer, adjusted the prototype for u_int16_t and size_t, and
replaced the fatal with a printf and return -1. Why -1? Well, the
checks always do if (in_cksum() != 0) { error(); }. Unless they
create a packet, but I guess in that case I hope we don't create a
packet that's too big *in our bootloader*.
Since the implementation was replaced, taken from ospfd, I guess the
4th-clause can go as well? I mean, there's no 4th-clause in ospfd and
I just copied the file.
This fixes my issue and works for me.
Opinions? ok?
Patrick
diff --git a/sys/lib/libsa/in_cksum.c b/sys/lib/libsa/in_cksum.c
index d3f2e6ac978..c4b12e01c04 100644
--- a/sys/lib/libsa/in_cksum.c
+++ b/sys/lib/libsa/in_cksum.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: in_cksum.c,v 1.5 2014/11/19 20:28:56 miod Exp $ */
+/* $OpenBSD: in_cksum.c,v 1.7 2014/07/20 20:27:19 tobias Exp $ */
/* $NetBSD: in_cksum.c,v 1.3 1995/04/22 13:53:48 cgd Exp $ */
/*
@@ -17,11 +17,7 @@
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
- * 3. All advertising materials mentioning features or use of this software
- * must display the following acknowledgement:
- * This product includes software developed by the University of
- * California, Lawrence Berkeley Laboratory and its contributors.
- * 4. Neither the name of the University nor the names of its contributors
+ * 3. Neither the name of the University nor the names of its contributors
* may be used to endorse or promote products derived from this software
* without specific prior written permission.
*
@@ -56,35 +52,38 @@
* code and should be modified for each CPU to be as fast as possible.
* In particular, it should not be this one.
*/
-int
-in_cksum(const void *p, int len)
+u_int16_t
+in_cksum(const void *p, size_t l)
{
- int sum = 0, oddbyte = 0, v = 0;
+ unsigned int sum = 0;
+ int len;
const u_char *cp = p;
- /* we assume < 2^16 bytes being summed */
- while (len > 0) {
- if (oddbyte) {
- sum += v + *cp++;
- len--;
+ /* ensure that < 2^16 bytes being summed */
+ if (l >= (1 << 16)) {
+ printf("in_cksum: packet too big\n");
+ return -1;
+ }
+ len = (int)l;
+
+ if (((long)cp & 1) == 0) {
+ while (len > 1) {
+ sum += htons(*(u_short *)cp);
+ cp += 2;
+ len -= 2;
}
- if (((long)cp & 1) == 0) {
- while ((len -= 2) >= 0) {
- sum += *(const u_short *)cp;
- cp += 2;
- }
- } else {
- while ((len -= 2) >= 0) {
- sum += *cp++ << 8;
- sum += *cp++;
- }
+ } else {
+ while (len > 1) {
+ sum += *cp++ << 8;
+ sum += *cp++;
+ len -= 2;
}
- if ((oddbyte = len & 1) != 0)
- v = *cp << 8;
}
- if (oddbyte)
- sum += v;
+ if (len == 1)
+ sum += *cp << 8;
+
sum = (sum >> 16) + (sum & 0xffff); /* add in accumulated carries */
sum += sum >> 16; /* add potential last carry */
+ sum = ntohs(sum);
return (0xffff & ~sum);
}
diff --git a/sys/lib/libsa/net.h b/sys/lib/libsa/net.h
index 48e21dadf17..1d3cc60793e 100644
--- a/sys/lib/libsa/net.h
+++ b/sys/lib/libsa/net.h
@@ -115,7 +115,7 @@ ssize_t sendrecv(struct iodesc *,
/* Utilities: */
const char *ether_sprintf(const u_char *);
-int in_cksum(const void *, int);
+u_int16_t in_cksum(const void *, size_t);
const char *inet_ntoa(struct in_addr);
const char *intoa(u_int32_t); /* similar to inet_ntoa */
u_int32_t inet_addr(const char *);
