From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 22D2A1DFFB; Thu, 15 Aug 2024 14:37:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723732625; cv=none; b=OBSU9/zFEUGfGkn4Rjsr+YNa4KE3jvKaVFinAfxHkbDatgSGFF/kmow88B1wPluUVUEqssRT7Sb9+xeeX9ns2lkBjEKcDjHxf4IGJVmHEW3OlrUIiKfwMiKVwyS5JPBYbts03J/vLxCCV4ZLinxQnBELa35L3iszM8q2xVHInyk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723732625; c=relaxed/simple; bh=7Wmd0g0AiW1Tfw0Uu7OAUIsfvl0klj8dFWHJbeRNnVc=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Oq76aUAXaC31Odcws3iCBnsPErkM+dHcrmJHWza80whH1Lcmv9vCA2bHw2zF5HBFZ3O42CZipP5x78clpjpDjoa5gZoLl63zBIQlV+sjMvNZ+OWPMYLZwDKc4tdk1Pa/WmBuUIbJ6HmtT0bJTmuzC08rC4QxU7rnhFWVjrXCrKM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=htIQMQuQ; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="htIQMQuQ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 972FAC32786; Thu, 15 Aug 2024 14:37:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1723732625; bh=7Wmd0g0AiW1Tfw0Uu7OAUIsfvl0klj8dFWHJbeRNnVc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=htIQMQuQoXyWikHJI69WvMWQfZm4l3UF9NKLFKoQC3nZtoonqw8nXEM3YRJdrPpkJ JPjMfIpr1XE4F3MB4/4bMY+16+Fm5A3EFaJRHomDnYErhoNEh1xSjBP5DHaUl9ss6g bmLC4BFcI4OmNZy9iS0wELdlPXKBu/VH2vClTWlg= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Halil Pasic , Alexandra Winter , Paolo Abeni , Sasha Levin Subject: [PATCH 5.10 236/352] net/iucv: fix use after free in iucv_sock_close() Date: Thu, 15 Aug 2024 15:25:02 +0200 Message-ID: <20240815131928.558592869@linuxfoundation.org> X-Mailer: git-send-email 2.46.0 In-Reply-To: <20240815131919.196120297@linuxfoundation.org> References: <20240815131919.196120297@linuxfoundation.org> User-Agent: quilt/0.67 X-stable: review X-Patchwork-Hint: ignore Precedence: bulk X-Mailing-List: patches@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 5.10-stable review patch. If anyone has any objections, please let me know. ------------------ From: Alexandra Winter [ Upstream commit f558120cd709682b739207b48cf7479fd9568431 ] iucv_sever_path() is called from process context and from bh context. iucv->path is used as indicator whether somebody else is taking care of severing the path (or it is already removed / never existed). This needs to be done with atomic compare and swap, otherwise there is a small window where iucv_sock_close() will try to work with a path that has already been severed and freed by iucv_callback_connrej() called by iucv_tasklet_fn(). Example: [452744.123844] Call Trace: [452744.123845] ([<0000001e87f03880>] 0x1e87f03880) [452744.123966] [<00000000d593001e>] iucv_path_sever+0x96/0x138 [452744.124330] [<000003ff801ddbca>] iucv_sever_path+0xc2/0xd0 [af_iucv] [452744.124336] [<000003ff801e01b6>] iucv_sock_close+0xa6/0x310 [af_iucv] [452744.124341] [<000003ff801e08cc>] iucv_sock_release+0x3c/0xd0 [af_iucv] [452744.124345] [<00000000d574794e>] __sock_release+0x5e/0xe8 [452744.124815] [<00000000d5747a0c>] sock_close+0x34/0x48 [452744.124820] [<00000000d5421642>] __fput+0xba/0x268 [452744.124826] [<00000000d51b382c>] task_work_run+0xbc/0xf0 [452744.124832] [<00000000d5145710>] do_notify_resume+0x88/0x90 [452744.124841] [<00000000d5978096>] system_call+0xe2/0x2c8 [452744.125319] Last Breaking-Event-Address: [452744.125321] [<00000000d5930018>] iucv_path_sever+0x90/0x138 [452744.125324] [452744.125325] Kernel panic - not syncing: Fatal exception in interrupt Note that bh_lock_sock() is not serializing the tasklet context against process context, because the check for sock_owned_by_user() and corresponding handling is missing. Ideas for a future clean-up patch: A) Correct usage of bh_lock_sock() in tasklet context, as described in Link: https://lore.kernel.org/netdev/1280155406.2899.407.camel@edumazet-laptop/ Re-enqueue, if needed. This may require adding return values to the tasklet functions and thus changes to all users of iucv. B) Change iucv tasklet into worker and use only lock_sock() in af_iucv. Fixes: 7d316b945352 ("af_iucv: remove IUCV-pathes completely") Reviewed-by: Halil Pasic Signed-off-by: Alexandra Winter Link: https://patch.msgid.link/20240729122818.947756-1-wintera@linux.ibm.com Signed-off-by: Paolo Abeni Signed-off-by: Sasha Levin --- net/iucv/af_iucv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c index 7c73faa5336cd..3d0424e4ae6c9 100644 --- a/net/iucv/af_iucv.c +++ b/net/iucv/af_iucv.c @@ -359,8 +359,8 @@ static void iucv_sever_path(struct sock *sk, int with_user_data) struct iucv_sock *iucv = iucv_sk(sk); struct iucv_path *path = iucv->path; - if (iucv->path) { - iucv->path = NULL; + /* Whoever resets the path pointer, must sever and free it. */ + if (xchg(&iucv->path, NULL)) { if (with_user_data) { low_nmcpy(user_data, iucv->src_name); high_nmcpy(user_data, iucv->dst_name); -- 2.43.0