From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.katalix.com (mail.katalix.com [3.9.82.81]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 3554D282E1; Wed, 3 Jul 2024 16:37:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=3.9.82.81 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720024635; cv=none; b=Cm8C/FpAEr4Uc2RW3+GxiqMG8yjMgiHfixoKUabFEJtj9Rm41Cms1xQJpvqsbB7tY+T3RI2VBJ0KwzZwK3cwSSqANBlPBBN1CcyVs+dTMANMINBqbdAgHaKV+Id7HxAWTq14vdyKcQEHuu5ZlPm3HKeo2y2aHJ4n+Ur8osUSXXY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720024635; c=relaxed/simple; bh=2y8Ioe1+2tFcUJ6XUBHrn0SZoz2c6uK9QgFNxcnIyu0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=YI/0qtRg6Jyv5HYmlSz3vqgpxVIS9ovmhOWzfZHRGsBdglK+zMgPM6T13l2d4GkiHSthnIL4lcenaBpoFSww2yT3hxb1mX+O3wCXDCBPnTbwo3fZlHcnrxz4S5GTBVs6y5G1vcsH1SxXgA6dUhuVR/jzcHQiouVEk+bIfgPTgp4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=katalix.com; spf=pass smtp.mailfrom=katalix.com; dkim=pass (2048-bit key) header.d=katalix.com header.i=@katalix.com header.b=logI4Hsv; arc=none smtp.client-ip=3.9.82.81 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=katalix.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=katalix.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=katalix.com header.i=@katalix.com header.b="logI4Hsv" Received: from localhost (unknown [IPv6:2a02:8012:909b:0:1e90:7398:2278:75e2]) (Authenticated sender: tom) by mail.katalix.com (Postfix) with ESMTPSA id 2C91E7D57F; Wed, 3 Jul 2024 17:37:12 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=katalix.com; s=mail; t=1720024632; bh=2y8Ioe1+2tFcUJ6XUBHrn0SZoz2c6uK9QgFNxcnIyu0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Disposition:In-Reply-To:From; z=Date:=20Wed,=203=20Jul=202024=2017:37:11=20+0100|From:=20Tom=20Pa rkin=20|To:=20syzbot=20|Cc:=20linux-kernel@vger.kernel.o rg,=20netdev@vger.kernel.org,=0D=0A=09syzkaller-bugs@googlegroups. com|Subject:=20Re:=20[syzbot]=20[net?]=20KASAN:=20slab-use-after-f ree=20Write=20in=0D=0A=20l2tp_session_delete|Message-ID:=20|References:=20<0000000000008405e0061bb6d4d 5@google.com>|MIME-Version:=201.0|Content-Disposition:=20inline|In -Reply-To:=20<0000000000008405e0061bb6d4d5@google.com>; b=logI4HsveAitLm2sLrWYknNbtNAbKK0ljzlC+AMu3SJviFW5WIVos3DE+UPKCcCZ7 MxL3YRe8dErL+PQmV8ONQ8iaFSmlZ8XovYIskuGzvlKAewhSzfyG2MtnM5TE6ez5ii sSs4PruZUg7/v2LbYWBh5rAcRnFprWUKI7gl3uDYH7c0G36SIhJcBuFvMP8HYwvwYV 4eO8Ik6mJxzc1iUOs3dAIW3mpBprT2fYKU6bPtAYvW2jQx7GSUPl4KlI7sFRf2DFAI u6ZDKXW0dR1m60d7HN2dcP9etpz+T3vIKgbXztpayXKNStgHMJowP02mOsDKfKeNCu 9cOoiXoe43vXg== Date: Wed, 3 Jul 2024 17:37:11 +0100 From: Tom Parkin To: syzbot Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, syzkaller-bugs@googlegroups.com Subject: Re: [syzbot] [net?] KASAN: slab-use-after-free Write in l2tp_session_delete Message-ID: References: <0000000000008405e0061bb6d4d5@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="xgA555V3jkRH7ho/" Content-Disposition: inline In-Reply-To: <0000000000008405e0061bb6d4d5@google.com> --xgA555V3jkRH7ho/ Content-Type: multipart/mixed; boundary="NltMTb9tJT1zWXBQ" Content-Disposition: inline --NltMTb9tJT1zWXBQ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jun 25, 2024 at 06:25:23 -0700, syzbot wrote: > syzbot found the following issue on: >=20 > HEAD commit: 185d72112b95 net: xilinx: axienet: Enable multicast by de= f.. > git tree: net-next > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=3D1062bd46980000 #syz test https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.g= it 185d72112b95 --NltMTb9tJT1zWXBQ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-l2tp-fix-possible-UAF-when-cleaning-up-tunnels.patch" Content-Transfer-Encoding: quoted-printable =46rom 6bc347af14eba3b37f9e8f0831a53ef1aae4c203 Mon Sep 17 00:00:00 2001 =46rom: Tom Parkin Date: Wed, 3 Jul 2024 17:32:00 +0100 Subject: [PATCH] l2tp: fix possible UAF when cleaning up tunnels syzbot reported a UAF caused by a race when the L2TP work queue closes a tunnel at the same time as a userspace thread closes a session in that tunnel. Tunnel cleanup is handled by a work queue which iterates through the sessions contained within a tunnel, and closes them in turn. Meanwhile, a userspace thread may arbitrarily close a session via either netlink command or by closing the pppox socket in the case of l2tp_ppp. The race condition may occur when l2tp_tunnel_closeall walks the list of sessions in the tunnel and deletes each one. Currently this is implemented using list_for_each_safe, but because the list spinlock is dropped in the loop body it's possible for other threads to manipulate the list during list_for_each_safe's list walk. This can lead to the list iterator being corrupted, leading to list_for_each_safe spinning. One sequence of events which may lead to this is as follows: * A tunnel is created, containing two sessions A and B. * A thread closes the tunnel, triggering tunnel cleanup via the work queue. * l2tp_tunnel_closeall runs in the context of the work queue. It removes session A from the tunnel session list, then drops the list lock. At this point the list_for_each_safe temporary variable is pointing to the other session on the list, which is session B, and the list can be manipulated by other threads since the list lock has been released. * Userspace closes session B, which removes the session from its parent tunnel via l2tp_session_delete. Since l2tp_tunnel_closeall has released the tunnel list lock, l2tp_session_delete is able to call list_del_init on the session B list node. * Back on the work queue, l2tp_tunnel_closeall resumes execution and will now spin forever on the same list entry until the underlying session structure is freed, at which point UAF occurs. The solution is to iterate over the tunnel's session list using list_first_entry_not_null to avoid the possibility of the list iterator pointing at a list item which may be removed during the walk. --- net/l2tp/l2tp_core.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index be4bcbf291a1..afa180b7b428 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1290,13 +1290,14 @@ 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 __rcu *pos; - struct list_head *tmp; =20 spin_lock_bh(&tunnel->list_lock); tunnel->acpt_newsess =3D false; - list_for_each_safe(pos, tmp, &tunnel->session_list) { - session =3D list_entry(pos, struct l2tp_session, list); + for (;;) { + session =3D list_first_entry_or_null(&tunnel->session_list, + struct l2tp_session, list); + if (!session) + break; list_del_init(&session->list); spin_unlock_bh(&tunnel->list_lock); l2tp_session_delete(session); --=20 2.34.1 --NltMTb9tJT1zWXBQ-- --xgA555V3jkRH7ho/ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEsUkgyDzMwrj81nq0lIwGZQq6i9AFAmaFfjMACgkQlIwGZQq6 i9DG5Af/czXZIGyxcypF5ucerzC/KmZqLog1ZIQSe4C9YYAjQQDPEz3n6gI/jdsC 3OlWbiSwdCK88je5FL4XLQ/QG73OOtI0v/jwXwinnUwEbBZY1s/JsjGqVlKF73Hm HVCIC+6TrL1D1jgZLnOWiOc8P2mXBNu/CpYOGlJOFZ9EgWF144K+AOgVPGZHFyNS iqWilTZSHi8CrQDewnqgsbTw1VDTGooX9BGAkj2JvTM0yr2YdEKFTdtie36hZavD 9O8++vaPHJMWOCrQDoNXx7/dUySpGead7ARYFLjjeDhhjZI+fVFXmaORFeNxTnyS qIB0myjp3kop4sQWmgvnp28HTuob/A== =l3VA -----END PGP SIGNATURE----- --xgA555V3jkRH7ho/--