Date:   Fri, 17 Feb 2023 12:14:34 +0100
From:   Guillaume Nault <gnault@redhat.com>
To:     Shigeru Yoshida <syoshida@redhat.com>
Cc:     jchapman@katalix.com, davem@davemloft.net, edumazet@google.com,
        kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, Cong Wang <cong.wang@bytedance.com>
Subject: Re: [PATCH net v3] l2tp: Avoid possible recursive deadlock in
 l2tp_tunnel_register()
Message-ID: <Y+9hmmAlHOo4yX9Q@debian>
References: <20230216163710.2508327-1-syoshida@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230216163710.2508327-1-syoshida@redhat.com>
Precedence: bulk

On Fri, Feb 17, 2023 at 01:37:10AM +0900, Shigeru Yoshida wrote:
> @@ -840,8 +850,7 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
>  	}
>  	if (drop_refcnt)
>  		l2tp_session_dec_refcount(session);
> -	if (drop_tunnel)
> -		l2tp_tunnel_dec_refcount(tunnel);
> +	l2tp_tunnel_dec_refcount(tunnel);
>  	release_sock(sk);
>  
>  	return error;

The l2tp_tunnel_dec_refcount() call could be done after release_sock(),
to make the code more logical (as the refcount is now taken before
lock_sock()). Anyway, that shouldn't be a problem and I don't want to
delay this fix any longer.

Reviewed-by: Guillaume Nault <gnault@redhat.com>