netdev - [PATCH 5/9] l2tp: don't BUG_ON session magic checks in l2tp

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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20200724153157.9366-6-tparkin@katalix.com>
Date:   Fri, 24 Jul 2020 16:31:53 +0100
From:   Tom Parkin <tparkin@...alix.com>
To:     netdev@...r.kernel.org
Cc:     jchapman@...alix.com, Tom Parkin <tparkin@...alix.com>
Subject: [PATCH 5/9] l2tp: don't BUG_ON session magic checks in l2tp_ppp

checkpatch advises that WARN_ON and recovery code are preferred over
BUG_ON which crashes the kernel.

l2tp_ppp.c's BUG_ON checks of the l2tp session structure's "magic" field
occur in code paths where it's reasonably easy to recover:

 * In the case of pppol2tp_sock_to_session, we can return NULL and the
   caller will bail out appropriately.  There is no change required to
   any of the callsites of this function since they already handle
   pppol2tp_sock_to_session returning NULL.

 * In the case of pppol2tp_session_destruct we can just avoid
   decrementing the reference count on the suspect session structure.
   In the worst case scenario this results in a memory leak, which is
   preferable to a crash.

Convert these uses of BUG_ON to WARN_ON accordingly.

Signed-off-by: Tom Parkin <tparkin@...alix.com>
---
 net/l2tp/l2tp_ppp.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index e58fe7e3b884..3b6613cfef48 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -163,8 +163,11 @@ static inline struct l2tp_session *pppol2tp_sock_to_session(struct sock *sk)
 		sock_put(sk);
 		goto out;
 	}
-
-	BUG_ON(session->magic != L2TP_SESSION_MAGIC);
+	if (WARN_ON(session->magic != L2TP_SESSION_MAGIC)) {
+		session = NULL;
+		sock_put(sk);
+		goto out;
+	}

 out:
 	return session;
@@ -419,7 +422,8 @@ static void pppol2tp_session_destruct(struct sock *sk)

 	if (session) {
 		sk->sk_user_data = NULL;
-		BUG_ON(session->magic != L2TP_SESSION_MAGIC);
+		if (WARN_ON(session->magic != L2TP_SESSION_MAGIC))
+			return;
 		l2tp_session_dec_refcount(session);
 	}
 }
-- 
2.17.1