From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1F46BC4332F for ; Sat, 24 Dec 2022 01:32:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236470AbiLXBcC (ORCPT ); Fri, 23 Dec 2022 20:32:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59058 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236454AbiLXBbT (ORCPT ); Fri, 23 Dec 2022 20:31:19 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0E3903054C; Fri, 23 Dec 2022 17:30:26 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id E7CCCB80A26; Sat, 24 Dec 2022 01:30:18 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 00DD7C433EF; Sat, 24 Dec 2022 01:30:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1671845417; bh=kZIdz+kwK19u4X7Jpo86YBXa80e8UIlL96mj+VX3Z3k=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=A9UGPOZAuRNRBxO91xeruQc7BkfMZyXtvswDZCurZM+Ay7GzITpUxBy2P0Jmb3Do0 sc7pOFXV+Hu0DYsfIM4iRcGk80bRuh5kuScGbjIQp6isJaCxcrpN2W7tRVhkz/CqOa /bIM6MudLgBVavuBl+lZm20J+OrYr7F5N+vw4q+M9V/gpfEK3ae5BzALvxTczHtJYR ClMGkb1X0NyE8stH3Qoph6AsFsBzf7LbJi2AdYNnVCEjbrpYLoCRuY8EsWn6iqZ33R lTdXm+BCMsLCLEMjP2XllCnrWKQrgYWqPrcT10eWw+JMQ+5I0g1QuB4xY2qrrehLka HZ/7i968zWQbg== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Eli Billauer , Hyunwoo Kim , Alan Stern , Greg Kroah-Hartman , Sasha Levin Subject: [PATCH AUTOSEL 6.1 17/26] char: xillybus: Prevent use-after-free due to race condition Date: Fri, 23 Dec 2022 20:29:21 -0500 Message-Id: <20221224012930.392358-17-sashal@kernel.org> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221224012930.392358-1-sashal@kernel.org> References: <20221224012930.392358-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Eli Billauer [ Upstream commit 282a4b71816b6076029017a7bab3a9dcee12a920 ] The driver for XillyUSB devices maintains a kref reference count on each xillyusb_dev structure, which represents a physical device. This reference count reaches zero when the device has been disconnected and there are no open file descriptors that are related to the device. When this occurs, kref_put() calls cleanup_dev(), which clears up the device's data, including the structure itself. However, when xillyusb_open() is called, this reference count becomes tricky: This function needs to obtain the xillyusb_dev structure that relates to the inode's major and minor (as there can be several such). xillybus_find_inode() (which is defined in xillybus_class.c) is called for this purpose. xillybus_find_inode() holds a mutex that is global in xillybus_class.c to protect the list of devices, and releases this mutex before returning. As a result, nothing protects the xillyusb_dev's reference counter from being decremented to zero before xillyusb_open() increments it on its own behalf. Hence the structure can be freed due to a rare race condition. To solve this, a mutex is added. It is locked by xillyusb_open() before the call to xillybus_find_inode() and is released only after the kref counter has been incremented on behalf of the newly opened inode. This protects the kref reference counters of all xillyusb_dev structs from being decremented by xillyusb_disconnect() during this time segment, as the call to kref_put() in this function is done with the same lock held. There is no need to hold the lock on other calls to kref_put(), because if xillybus_find_inode() finds a struct, xillyusb_disconnect() has not made the call to remove it, and hence not made its call to kref_put(), which takes place afterwards. Hence preventing xillyusb_disconnect's call to kref_put() is enough to ensure that the reference doesn't reach zero before it's incremented by xillyusb_open(). It would have been more natural to increment the reference count in xillybus_find_inode() of course, however this function is also called by Xillybus' driver for PCIe / OF, which registers a completely different structure. Therefore, xillybus_find_inode() treats these structures as void pointers, and accordingly can't make any changes. Reported-by: Hyunwoo Kim Suggested-by: Alan Stern Signed-off-by: Eli Billauer Link: https://lore.kernel.org/r/20221030094209.65916-1-eli.billauer@gmail.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin --- drivers/char/xillybus/xillyusb.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/drivers/char/xillybus/xillyusb.c b/drivers/char/xillybus/xillyusb.c index 39bcbfd908b4..5a5afa14ca8c 100644 --- a/drivers/char/xillybus/xillyusb.c +++ b/drivers/char/xillybus/xillyusb.c @@ -184,6 +184,14 @@ struct xillyusb_dev { struct mutex process_in_mutex; /* synchronize wakeup_all() */ }; +/* + * kref_mutex is used in xillyusb_open() to prevent the xillyusb_dev + * struct from being freed during the gap between being found by + * xillybus_find_inode() and having its reference count incremented. + */ + +static DEFINE_MUTEX(kref_mutex); + /* FPGA to host opcodes */ enum { OPCODE_DATA = 0, @@ -1237,9 +1245,16 @@ static int xillyusb_open(struct inode *inode, struct file *filp) int rc; int index; + mutex_lock(&kref_mutex); + rc = xillybus_find_inode(inode, (void **)&xdev, &index); - if (rc) + if (rc) { + mutex_unlock(&kref_mutex); return rc; + } + + kref_get(&xdev->kref); + mutex_unlock(&kref_mutex); chan = &xdev->channels[index]; filp->private_data = chan; @@ -1275,8 +1290,6 @@ static int xillyusb_open(struct inode *inode, struct file *filp) ((filp->f_mode & FMODE_WRITE) && chan->open_for_write)) goto unmutex_fail; - kref_get(&xdev->kref); - if (filp->f_mode & FMODE_READ) chan->open_for_read = 1; @@ -1413,6 +1426,7 @@ static int xillyusb_open(struct inode *inode, struct file *filp) return rc; unmutex_fail: + kref_put(&xdev->kref, cleanup_dev); mutex_unlock(&chan->lock); return rc; } @@ -2227,7 +2241,9 @@ static void xillyusb_disconnect(struct usb_interface *interface) xdev->dev = NULL; + mutex_lock(&kref_mutex); kref_put(&xdev->kref, cleanup_dev); + mutex_unlock(&kref_mutex); } static struct usb_driver xillyusb_driver = { -- 2.35.1