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 <tpar...@katalix.com>
---
 net/l2tp/l2tp_ppp.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index e58fe7e3b884..6cd1a422c426 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -163,8 +163,12 @@ 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 (session->magic != L2TP_SESSION_MAGIC) {
+               WARN_ON(session->magic != L2TP_SESSION_MAGIC);
+               session = NULL;
+               sock_put(sk);
+               goto out;
+       }
 
 out:
        return session;
@@ -419,8 +423,9 @@ static void pppol2tp_session_destruct(struct sock *sk)
 
        if (session) {
                sk->sk_user_data = NULL;
-               BUG_ON(session->magic != L2TP_SESSION_MAGIC);
-               l2tp_session_dec_refcount(session);
+               WARN_ON(session->magic != L2TP_SESSION_MAGIC);
+               if (session->magic == L2TP_SESSION_MAGIC)
+                       l2tp_session_dec_refcount(session);
        }
 }
 
-- 
2.17.1

Reply via email to