netdev - Re: [PATCH net-next v2] l2tp: fix possible UAF when cleaning up tunnels

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: <20240705103250.3144-1-hdanton@sina.com>
Date: Fri,  5 Jul 2024 18:32:50 +0800
From: Hillf Danton <hdanton@...a.com>
To: James Chapman <jchapman@...alix.com>
Cc: netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	syzkaller-bugs@...glegroups.com,
	tparkin@...alix.com,
	syzbot+b471b7c936301a59745b@...kaller.appspotmail.com,
	syzbot+c041b4ce3a6dfd1e63e2@...kaller.appspotmail.com
Subject: Re: [PATCH net-next v2] l2tp: fix possible UAF when cleaning up tunnels

On Thu,  4 Jul 2024 16:25:08 +0100 James Chapman <jchapman@...alix.com>
> --- a/net/l2tp/l2tp_core.c
> +++ b/net/l2tp/l2tp_core.c
> @@ -1290,17 +1290,20 @@ static void l2tp_session_unhash(struct l2tp_session *session)
>  static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel)
>  {
>  	struct l2tp_session *session;
> -	struct list_head *pos;
> -	struct list_head *tmp;
>  
>  	spin_lock_bh(&tunnel->list_lock);
>  	tunnel->acpt_newsess = false;
> -	list_for_each_safe(pos, tmp, &tunnel->session_list) {
> -		session = list_entry(pos, struct l2tp_session, list);
> +	for (;;) {
> +		session = list_first_entry_or_null(&tunnel->session_list,
> +						   struct l2tp_session, list);
> +		if (!session)
> +			break;
> +		l2tp_session_inc_refcount(session);
>  		list_del_init(&session->list);
>  		spin_unlock_bh(&tunnel->list_lock);
>  		l2tp_session_delete(session);
>  		spin_lock_bh(&tunnel->list_lock);
> +		l2tp_session_dec_refcount(session);

Bumping refcount up makes it safe for the current cpu to go thru race
after releasing lock, and if it wins the race, dropping refcount makes
the peer head on uaf.