Subject: Flaw in fastsum() in in_cksum.c
To: , <tech-net@netbsd.org>
From: Krejsa, Dan <dan.krejsa@windriver.com>
List: tech-net
Date: 03/24/2006 12:04:11
This is a multi-part message in MIME format.

------_=_NextPart_001_01C64F7E.1EA02741
Content-Type: text/plain;
	charset="us-ascii"
Content-Transfer-Encoding: quoted-printable

Hi,

There's a flaw in the fastsum() code in
src/sys/arch/mips/mips/in_cksum.c .

The code does not properly handle the case where one
of the mbuf data segments is exactly 2 bytes long and
starts at an address not divisible by 4.  In that case,
the following code at the 'verylittleleft' label
repeats the same byte twice in the checksum:

 verylittleleft:
        /* handle trailing byte and short (possibly) unaligned payloads
*/
        while (n-- > 0) {
#if BYTE_ORDER =3D=3D BIG_ENDIAN
                sum +=3D *buf.c << 8;
#else
                sum +=3D *buf.c;
#endif
        }

A diff between revisions 1.11 and a version incorporating
a fix and also some optimizations suggested by David Borman
is attached.

- Dan

P.S. Other ports may also be using a flawed fastsum(); I haven't
checked them all.


------_=_NextPart_001_01C64F7E.1EA02741
Content-Type: application/octet-stream;
	name="in_cksum.c.diff"
Content-Transfer-Encoding: base64
Content-Description: in_cksum.c.diff
Content-Disposition: attachment;
	filename="in_cksum.c.diff"

KioqIGluX2Nrc3VtLmMJMjAwNi0wMy0yMyAxNTo1ODoxMy4wMDAwMDAwMDAgLTA4MDAKLS0tIGlu
X2Nrc3VtLmMubmV3CTIwMDYtMDMtMjMgMTU6NTU6NTUuMDAwMDAwMDAwIC0wODAwCioqKioqKioq
KioqKioqKgoqKiogNzMsOTMgKioqKgogICAqICdvZGRfYWxpZ25lZCcgaXMgYSBib29sZWFuIHdo
aWNoIGlmIHNldCwgaW5kaWNhdGUgdGhlIGRhdGEgaW4gJ2J1ZicKICAgKiBzdGFydHMgYXQgYW4g
b2RkIGJ5dGUgYWxpZ25tZW50IHdpdGhpbiB0aGUgY29udGFpbmluZyBwYWNrZXQsCiAgICogYW5k
IHNvIHdlIG11c3QgYnl0ZXN3YXAgdGhlIG1lbW9yeS1hbGlnbmVkIDEncy1jb21wbGVtZW50IHN1
bQogICAqIG92ZXIgdGhlIGRhdGEgYmVmb3JlIGFkZGluZyBpdCB0byBgb2xkc3VtJy4KICAgKi8K
ICBzdGF0aWMgaW5saW5lIHVpbnQzMl90CiAgZmFzdHN1bSh1bmlvbiBtZW1wdHIgYnVmLCBpbnQg
biwgdW5zaWduZWQgaW50IG9sZHN1bSwgaW50IG9kZF9hbGlnbmVkKQogIHsKICAJdW5zaWduZWQg
bG9uZyBoaWxvID0gMCwgaGlnaCA9IDA7CiAgCXVuc2lnbmVkIGxvbmcgdzAsIHcxOwogIAl1bnNp
Z25lZCBpbnQgc3VtID0gMDsKICAKICAJLyogQWxpZ24gdG8gMzIgYml0cy4gKi8KICAJaWYgKGJ1
Zi51ICYgMHgzKSB7Ci0gCQkvKiBTa2lwIHRvIHRoZSBlbmQgZm9yIHZlcnkgc21hbGwgbWJ1ZnMg
Ki8KLSAJCWlmIChuIDwgMykKLSAJCQlnb3RvIHZlcnlsaXR0bGVsZWZ0OwogIAogIAkJLyoKICAJ
ICAgICAgICAgKiAxNi1iaXQtYWxpZ24uCiAgCQkgKiBJZiBidWYgaXMgb2RkLWJ5dGUtYWxpZ25l
ZCwgYWRkIHRoZSBieXRlIGFuZCB0b2dnbGUKLS0tIDczLDEwMSAtLS0tCiAgICogJ29kZF9hbGln
bmVkJyBpcyBhIGJvb2xlYW4gd2hpY2ggaWYgc2V0LCBpbmRpY2F0ZSB0aGUgZGF0YSBpbiAnYnVm
JwogICAqIHN0YXJ0cyBhdCBhbiBvZGQgYnl0ZSBhbGlnbm1lbnQgd2l0aGluIHRoZSBjb250YWlu
aW5nIHBhY2tldCwKICAgKiBhbmQgc28gd2UgbXVzdCBieXRlc3dhcCB0aGUgbWVtb3J5LWFsaWdu
ZWQgMSdzLWNvbXBsZW1lbnQgc3VtCiAgICogb3ZlciB0aGUgZGF0YSBiZWZvcmUgYWRkaW5nIGl0
IHRvIGBvbGRzdW0nLgorICAqCisgICogZmFzdHN1bSgpIG1heSBub3Qgd29yayBmb3IgY2hlY2tz
dW1taW5nIGJ1ZmZlcnMgb2YgbGVuZ3RoIGdyZWF0ZXIKKyAgKiB0aGFuIDEyOEtCIGR1ZSB0byB0
aGUgcG9zc2liaWxpdHkgb2YgY2FycmllcyBhY2N1bXVsYXRpbmcgdG8gbW9yZQorICAqIHRoYW4g
MTYgYml0cywgYW5kIHRoZSAnY2FycmllcyBmcm9tIHRoZSBjYXJyaWVzJyBiZWluZyBkcm9wcGVk
LgorICAqIEZvciBpbnN0YW5jZSwgJ2hpZ2gnIGNvdWxkIG92ZXJmbG93LCBvciBjYXJyaWVzIGZy
b20gdGhlIGxvdyBwYXJ0CisgICogb2YgJ2hpbG8nIGNvdWxkIGFjY3VtdWxhdGUgZW5vdWdoIHRv
IGNhdXNlIGFkZGl0aW9uYWwgY2FycmllcyBvdXQgb2YgdGhlCisgICogaGlnaCBwYXJ0IG9mICdo
aWxvJywgbm90IGFjY291bnRlZCBmb3IgaW4gdGhlIGhpZ2ggcGFydCBvZiAnaGlnaCcuCisgICog
T2YgY291cnNlLCBJUHY0IGRhdGFncmFtcyBvZiBzaXplID49IDY0S0IgYXJlIG5vdCBwb3NzaWJs
ZSwgYW5kIElQdjYKKyAgKiBwcmVzZW50bHkgdXNlcyBhIGRpZmZlcmVudCBjb2RlIHBhdGguCiAg
ICovCiAgc3RhdGljIGlubGluZSB1aW50MzJfdAogIGZhc3RzdW0odW5pb24gbWVtcHRyIGJ1Ziwg
aW50IG4sIHVuc2lnbmVkIGludCBvbGRzdW0sIGludCBvZGRfYWxpZ25lZCkKICB7CiAgCXVuc2ln
bmVkIGxvbmcgaGlsbyA9IDAsIGhpZ2ggPSAwOwogIAl1bnNpZ25lZCBsb25nIHcwLCB3MTsKICAJ
dW5zaWduZWQgaW50IHN1bSA9IDA7CiAgCisgCS8qIEFzc2VydDogbiA+IDAgKi8KKyAKICAJLyog
QWxpZ24gdG8gMzIgYml0cy4gKi8KICAJaWYgKGJ1Zi51ICYgMHgzKSB7CiAgCiAgCQkvKgogIAkg
ICAgICAgICAqIDE2LWJpdC1hbGlnbi4KICAJCSAqIElmIGJ1ZiBpcyBvZGQtYnl0ZS1hbGlnbmVk
LCBhZGQgdGhlIGJ5dGUgYW5kIHRvZ2dsZQoqKioqKioqKioqKioqKioKKioqIDk5LDEwNiAqKioq
Ci0tLSAxMDcsMTE1IC0tLS0KICAJCSAqIGFuZCB3ZSBtdXN0IGJ5dGVzd2FwIG91ciAxNi1iaXQt
YWxpZ25lZCBzdW0gb2YKICAJCSAqJ2J1ZicgYmVmb3JlIGFjY3VtdWxhdGluZyBpdC4KICAJCSAq
LwogIAkJaWYgKGJ1Zi51ICYgMHgxKSB7CisgCQkJLyogTk9URTogV2UgYWx3YXlzIGhhdmUgYXQg
bGVhc3Qgb25lIGJ5dGUgKi8KICAjaWYgQllURV9PUkRFUiA9PSBCSUdfRU5ESUFOCiAgCQkJc3Vt
ICs9ICooYnVmLmMrKyk7CiAgI2Vsc2UKICAJCQlzdW0gKz0gKCooYnVmLmMrKykgPDwgOCk7Cioq
KioqKioqKioqKioqKgoqKiogMTEwLDExNyAqKioqCi0tLSAxMTksMTMwIC0tLS0KICAJCX0KICAK
ICAJCS8qIDMyLWJpdC1hbGlnbiAqLwogIAkJaWYgKGJ1Zi51ICYgMHgyKSB7CisgCQkJLyogU2tp
cCB0byB0aGUgZW5kIGZvciB2ZXJ5IHNtYWxsIG1idWZzICovCisgCQkJaWYgKG4gPCAyKQorIAkJ
CQlnb3RvIHZlcnlsaXR0bGVsZWZ0OworIAogIAkJCXN1bSArPSAqKGJ1Zi5zKyspOwogIAkJCW4g
LT0gMjsKICAJCX0KICAJfQoqKioqKioqKioqKioqKioKKioqIDIxNCwyMjMgKioqKgogIAkJc3Vt
ICs9ICooYnVmLnMrKyk7CiAgCX0KICAKICAgdmVyeWxpdHRsZWxlZnQ6CiEgCS8qIGhhbmRsZSB0
cmFpbGluZyBieXRlIGFuZCBzaG9ydCAocG9zc2libHkpIHVuYWxpZ25lZCBwYXlsb2FkcyAqLwoh
IAl3aGlsZSAobi0tID4gMCkgewogICNpZiBCWVRFX09SREVSID09IEJJR19FTkRJQU4KICAJCXN1
bSArPSAqYnVmLmMgPDwgODsKICAjZWxzZQogIAkJc3VtICs9ICpidWYuYzsKLS0tIDIyNywyMzcg
LS0tLQogIAkJc3VtICs9ICooYnVmLnMrKyk7CiAgCX0KICAKICAgdmVyeWxpdHRsZWxlZnQ6CiEg
CiEgCS8qIEF0IHRoaXMgcG9pbnQgbiBpcyAwIG9yIDEuIEhhbmRsZSB0aGUgdHJhaWxpbmcgYnl0
ZSwgaWYgcHJlc2VudC4gKi8KISAJaWYgKG4gPiAwKSB7CiAgI2lmIEJZVEVfT1JERVIgPT0gQklH
X0VORElBTgogIAkJc3VtICs9ICpidWYuYyA8PCA4OwogICNlbHNlCiAgCQlzdW0gKz0gKmJ1Zi5j
OwoqKioqKioqKioqKioqKioKKioqIDIyNywyNDYgKioqKgogIAkvKgogIAkgKiBjb21wZW5zYXRl
IGZvciBhIHRyYWlsaW5nIGJ5dGUgaW4gcHJldmlvdXMgbWJ1ZgogIAkgKiBieSBieXRlc3dhcHBp
bmcgdGhlIG1lbW9yeS1hbGlnbmVkIHN1bSBvZiB0aGlzIG1idWYuCiAgIAkgKi8KISAJaWYgKG9k
ZF9hbGlnbmVkKSB7CiEgCQlzdW0gPSAoc3VtICYgMHhmZmZmKSArIChzdW0gPj4gMTYpOwohIAkJ
c3VtID0gKHN1bSAmIDB4ZmZmZikgKyAoc3VtID4+IDE2KTsKISAJCXN1bSA9IG9sZHN1bSArICgo
c3VtID4+IDgpICYgMHhmZikgKyAoKHN1bSAmIDB4ZmYpIDw8IDgpOwohIAl9IGVsc2UgewohIAkJ
LyogYWRkIHVwcGVyIGFuZCBsb3dlciBoYWxmd29yZHMgdG9nZXRoZXIgdG8gZ2V0IGZ1bGwgc3Vt
ICovCiEgCQlzdW0gPSBvbGRzdW0gKyBzdW07CiEgCQlzdW0gPSAoc3VtICYgMHhmZmZmKSArIChz
dW0gPj4gMTYpOwohIAl9CiAgCi0gCS8qIGZvbGQgY2FycnkgZnJvbSBjb21iaW5pbmcgc3VtcyAq
LwotIAlzdW0gPSAoc3VtICYgMHhmZmZmKSArIChzdW0gPj4gMTYpOwogIAlyZXR1cm4oc3VtKTsK
ICB9CiAgCiAgLyoKLS0tIDI0MSwyNTYgLS0tLQogIAkvKgogIAkgKiBjb21wZW5zYXRlIGZvciBh
IHRyYWlsaW5nIGJ5dGUgaW4gcHJldmlvdXMgbWJ1ZgogIAkgKiBieSBieXRlc3dhcHBpbmcgdGhl
IG1lbW9yeS1hbGlnbmVkIHN1bSBvZiB0aGlzIG1idWYuCiAgIAkgKi8KISAJaWYgKG9kZF9hbGln
bmVkKQohIAkJc3VtID0gKHN1bSA+PiA4KSArICgoc3VtICYgMHhmZikgPDwgOCk7CiEgCiEgCXN1
bSArPSBvbGRzdW07CiEgCiEgCXN1bSA9IChzdW0gJiAweGZmZmYpICsgKHN1bSA+PiAxNik7CS8q
IEZvbGQgdG8gMTcgYml0cyAqLwohIAlzdW0gPSAoc3VtICYgMHhmZmZmKSArIChzdW0gPj4gMTYp
OwkvKiBGb2xkIGFnYWluIHRvIDE2IGJpdHMgKi8KICAKICAJcmV0dXJuKHN1bSk7CiAgfQogIAog
IC8qCg==

------_=_NextPart_001_01C64F7E.1EA02741--