Description Mikhail Teterin 2016-02-18 19:35:20 UTC

Created attachment 167155 [details]
Remove a silly check, const-poison key-handling

The article at

  http://www.viva64.com/en/b/0377/

listed a problem with the KAME-derived code: the key_parse() function is comparing m->m_pkthdr.len with itself. We have this line since 2002, when sam committed what was than known as FAST_IPSEC option in base r105197.

The original KAME sources (https://github.com/kame/kame/) and NetBSD have this issue, but I could not find this code in OpenBSD cvs-repo online.

The minimal fix is to simply remove the useless check -- something the compiler must've been doing automatically ever since:

@@ -7245,9 +7245,8 @@ key_parse(struct mbuf *m, struct socket
        orglen = PFKEY_UNUNIT64(msg->sadb_msg_len);
        target = KEY_SENDUP_ONE;

-       if ((m->m_flags & M_PKTHDR) == 0 ||
-           m->m_pkthdr.len != m->m_pkthdr.len) {
-               ipseclog((LOG_DEBUG, "%s: invalid message length.\n",__func__));
+       if ((m->m_flags & M_PKTHDR) == 0) {
+               ipseclog((LOG_DEBUG, "%s: invalid message length.\n", __func__));
                PFKEYSTAT_INC(out_invlen);
                error = EINVAL;
                goto senderror;

However, the attached patch goes further and adds "const-poisoning" to functions in netipsec/key.c and netipsec/keysock.c . Please, review.