linux-kernel - [PATCH net] net: l2tp: reduce log level when passing up invalid packets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 19 Feb 2021 20:06:15 +0100
From:   Matthias Schiffer <mschiffer@...verse-factory.net>
To:     netdev@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Tom Parkin <tparkin@...alix.com>, linux-kernel@...r.kernel.org,
        Matthias Schiffer <mschiffer@...verse-factory.net>
Subject: [PATCH net] net: l2tp: reduce log level when passing up invalid packets

Before commit 5ee759cda51b ("l2tp: use standard API for warning log
messages"), it was possible for userspace applications to use their own
control protocols on the backing sockets of an L2TP kernel device, and as
long as a packet didn't look like a proper L2TP data packet, it would be
passed up to userspace just fine.

After the mentioned change, this approach would lead to significant log
spam, as the previously hidden warnings are now shown by default. Not
even setting the T flag on the custom control packets is sufficient to
surpress these warnings, as packet length and L2TP version are checked
before the T flag.

Reduce all warnings debug level when packets are passed to userspace.

Fixes: 5ee759cda51b ("l2tp: use standard API for warning log messages")
Signed-off-by: Matthias Schiffer <mschiffer@...verse-factory.net>
---

I'm unsure what to do about the pr_warn_ratelimited() in
l2tp_recv_common(). It feels wrong to me that an incoming network packet
can trigger a kernel message above debug level at all, so maybe they
should be downgraded as well? I believe the only reason these were ever
warnings is that they were not shown by default.

 net/l2tp/l2tp_core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 7be5103ff2a8..40852488c62a 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -809,8 +809,8 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)

 	/* Short packet? */
 	if (!pskb_may_pull(skb, L2TP_HDR_SIZE_MAX)) {
-		pr_warn_ratelimited("%s: recv short packet (len=%d)\n",
-				    tunnel->name, skb->len);
+		pr_debug_ratelimited("%s: recv short packet (len=%d)\n",
+				     tunnel->name, skb->len);
 		goto error;
 	}

@@ -824,8 +824,8 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
 	/* Check protocol version */
 	version = hdrflags & L2TP_HDR_VER_MASK;
 	if (version != tunnel->version) {
-		pr_warn_ratelimited("%s: recv protocol version mismatch: got %d expected %d\n",
-				    tunnel->name, version, tunnel->version);
+		pr_debug_ratelimited("%s: recv protocol version mismatch: got %d expected %d\n",
+				     tunnel->name, version, tunnel->version);
 		goto error;
 	}

@@ -863,8 +863,8 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
 			l2tp_session_dec_refcount(session);

 		/* Not found? Pass to userspace to deal with */
-		pr_warn_ratelimited("%s: no session found (%u/%u). Passing up.\n",
-				    tunnel->name, tunnel_id, session_id);
+		pr_debug_ratelimited("%s: no session found (%u/%u). Passing up.\n",
+				     tunnel->name, tunnel_id, session_id);
 		goto error;
 	}

-- 
2.30.1