l2tp: remove l2specific_len dependency in l2tp_core

commit 62e7b6a57c7b9bf3c6fd99418eeec05b08a85c38 upstream.

Remove l2specific_len dependency while building l2tpv3 header or
parsing the received frame since default L2-Specific Sublayer is
always four bytes long and we don't need to rely on a user supplied
value.
Moreover in l2tp netlink code there are no sanity checks to
enforce the relation between l2specific_len and l2specific_type,
so sending a malformed netlink message is possible to set
l2specific_type to L2TP_L2SPECTYPE_DEFAULT (or even
L2TP_L2SPECTYPE_NONE) and set l2specific_len to a value greater than
4 leaking memory on the wire and sending corrupted frames.

Reviewed-by: Guillaume Nault <g.nault@alphalink.fr>
Tested-by: Guillaume Nault <g.nault@alphalink.fr>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 4b49e66..a8c3856 100644 (file)
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -704,11 +704,9 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
                                 "%s: recv data ns=%u, session nr=%u\n",
                                 session->name, ns, session->nr);
                }
+               ptr += 4;
        }
 
-       /* Advance past L2-specific header, if present */
-       ptr += session->l2specific_len;
-
        if (L2TP_SKB_CB(skb)->has_seq) {
                /* Received a packet with sequence numbers. If we're the LNS,
                 * check if we sre sending sequence numbers and if not,
@@ -1030,21 +1028,20 @@ static int l2tp_build_l2tpv3_header(struct l2tp_session *session, void *buf)
                memcpy(bufp, &session->cookie[0], session->cookie_len);
                bufp += session->cookie_len;
        }
-       if (session->l2specific_len) {
-               if (session->l2specific_type == L2TP_L2SPECTYPE_DEFAULT) {
-                       u32 l2h = 0;
-                       if (session->send_seq) {
-                               l2h = 0x40000000 | session->ns;
-                               session->ns++;
-                               session->ns &= 0xffffff;
-                               l2tp_dbg(session, L2TP_MSG_SEQ,
-                                        "%s: updated ns to %u\n",
-                                        session->name, session->ns);
-                       }
+       if (session->l2specific_type == L2TP_L2SPECTYPE_DEFAULT) {
+               u32 l2h = 0;
 
-                       *((__be32 *) bufp) = htonl(l2h);
+               if (session->send_seq) {
+                       l2h = 0x40000000 | session->ns;
+                       session->ns++;
+                       session->ns &= 0xffffff;
+                       l2tp_dbg(session, L2TP_MSG_SEQ,
+                                "%s: updated ns to %u\n",
+                                session->name, session->ns);
                }
-               bufp += session->l2specific_len;
+
+               *((__be32 *)bufp) = htonl(l2h);
+               bufp += 4;
        }
        if (session->offset)
                bufp += session->offset;
@@ -1723,7 +1720,7 @@ int l2tp_session_delete(struct l2tp_session *session)
 EXPORT_SYMBOL_GPL(l2tp_session_delete);
 
 /* We come here whenever a session's send_seq, cookie_len or
- * l2specific_len parameters are set.
+ * l2specific_type parameters are set.
  */
 void l2tp_session_set_header_len(struct l2tp_session *session, int version)
 {
@@ -1732,7 +1729,8 @@ void l2tp_session_set_header_len(struct l2tp_session *session, int version)
                if (session->send_seq)
                        session->hdr_len += 4;
        } else {
-               session->hdr_len = 4 + session->cookie_len + session->l2specific_len + session->offset;
+               session->hdr_len = 4 + session->cookie_len + session->offset;
+               session->hdr_len += l2tp_get_l2specific_len(session);
                if (session->tunnel->encap == L2TP_ENCAPTYPE_UDP)
                        session->hdr_len += 4;
        }

diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 9cf5468..3cb3fc6 100644 (file)
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -313,6 +313,17 @@ do {                                                                       \
 #define l2tp_session_dec_refcount(s) l2tp_session_dec_refcount_1(s)
 #endif
 
+static inline int l2tp_get_l2specific_len(struct l2tp_session *session)
+{
+       switch (session->l2specific_type) {
+       case L2TP_L2SPECTYPE_DEFAULT:
+               return 4;
+       case L2TP_L2SPECTYPE_NONE:
+       default:
+               return 0;
+       }
+}
+
 #define l2tp_printk(ptr, type, func, fmt, ...)                         \
 do {                                                                   \
        if (((ptr)->debug) & (type))                                    \