Received: by 2002:a05:6a10:2726:0:0:0:0 with SMTP id ib38csp652481pxb; Tue, 5 Apr 2022 17:23:48 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyqJKBzo1eARqMTbq2+VMqM0Qj8Acks9O0Va1kjiwFnmdO7Vwjy64sdNVSEgJO8GLqei7fy X-Received: by 2002:a17:906:9c82:b0:6e1:1d6c:914c with SMTP id fj2-20020a1709069c8200b006e11d6c914cmr5790165ejc.769.1649204506638; Tue, 05 Apr 2022 17:21:46 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1649204506; cv=none; d=google.com; s=arc-20160816; b=hLTQtmmC1OCO5nVb2WyGn6q+z0/0hTcGaffHSj1sG6r0ZG2TScX9Nyh4AuJJfeeBdX S8D5Sxzzgqdr6qSrKo9vnvoBh2VA8tk22XzmjU2oiNdqsc6vBE6EMjc5Xsa5HF9+OhoQ +jdR4zZ35hOx26g51/FHiJv2AzxQM3VrmYqMvbJcmvHGXCvGkEpIaomtB8zMCipNnCt5 FW+rglaWEFKBC60mwK8DfO152lYBqKU7SVWU/8SVmPRIM4+d5/BBBeMjE9w/S9AX5FiP qAqIf9pwqnJzd3zehVkR0E4MttKBA54od1x7VJlFMd3YfaUVSpKv3AJ3ehTl2ydl49JZ QUSg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=23iT/GHcTQ9KIfkj3PuxrjSbk8qtFeH7h/L63Vm4W6U=; b=N0uK72WLSFhHLSKI5zIHjJOAvWQh5iK+hOo9IrZsoLWp0hipV5YA/HKFMnwdBDp8Yv M9teUjJQaItypOzpadYH2hgBH1UkIlpPP+5AY/rt4LUvsoD5TsH6E7MantfU2W/gSxlK Vtq7b57awFSFB9OUGjkTC7zb/wHSW8nYdEoyLuQFnHYgrUcFngVWgzP6abz/0ytHq23q d7iLYHuAM/4uSouaATsifHUXQnIkqDs9pbuWZMSdoZdBii0tN6ltO4zlZAMJZBW+Bu9O 86qQmiWEWoTJz7bbtEKS6AQOAxsmHoGq+QgDqLFa6vkCwivIG1TQk1M23+EPcNj1q/fK OgmA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=imTQqj3G; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id u26-20020a170906b11a00b006dfad24c60csi10531136ejy.249.2022.04.05.17.21.21; Tue, 05 Apr 2022 17:21:46 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=imTQqj3G; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1383482AbiDENf2 (ORCPT + 99 others); Tue, 5 Apr 2022 09:35:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38790 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1347076AbiDEJY6 (ORCPT ); Tue, 5 Apr 2022 05:24:58 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E0E65A9940; Tue, 5 Apr 2022 02:14:46 -0700 (PDT) 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 dfw.source.kernel.org (Postfix) with ESMTPS id C9FA761674; Tue, 5 Apr 2022 09:14:43 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D609FC385A3; Tue, 5 Apr 2022 09:14:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1649150083; bh=APJ33ZcyptEY6LH63toIiRLFoQnqhYnaWleZfgvO5SQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=imTQqj3G5sU0lKFGYpJJPDBDA3ssyM13kVzIXXCFVnW9arXDBvOfwWc6vqlpWQ0fo CYSToS7cJjCfcaVpc934Qdg3t48rIavhVsHIW1nfcIp9oDR1czkkeD5zIIN15v+Tfp BBrEk0UQtCYfrZBnsDQdYSOdY3THv1/3Jv2Gvuf8= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Hulk Robot , Baokun Li , Richard Weinberger Subject: [PATCH 5.16 0947/1017] ubi: Fix race condition between ctrl_cdev_ioctl and ubi_cdev_ioctl Date: Tue, 5 Apr 2022 09:31:00 +0200 Message-Id: <20220405070422.322895584@linuxfoundation.org> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220405070354.155796697@linuxfoundation.org> References: <20220405070354.155796697@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Baokun Li commit 3cbf0e392f173ba0ce425968c8374a6aa3e90f2e upstream. Hulk Robot reported a KASAN report about use-after-free: ================================================================== BUG: KASAN: use-after-free in __list_del_entry_valid+0x13d/0x160 Read of size 8 at addr ffff888035e37d98 by task ubiattach/1385 [...] Call Trace: klist_dec_and_del+0xa7/0x4a0 klist_put+0xc7/0x1a0 device_del+0x4d4/0xed0 cdev_device_del+0x1a/0x80 ubi_attach_mtd_dev+0x2951/0x34b0 [ubi] ctrl_cdev_ioctl+0x286/0x2f0 [ubi] Allocated by task 1414: device_add+0x60a/0x18b0 cdev_device_add+0x103/0x170 ubi_create_volume+0x1118/0x1a10 [ubi] ubi_cdev_ioctl+0xb7f/0x1ba0 [ubi] Freed by task 1385: cdev_device_del+0x1a/0x80 ubi_remove_volume+0x438/0x6c0 [ubi] ubi_cdev_ioctl+0xbf4/0x1ba0 [ubi] [...] ================================================================== The lock held by ctrl_cdev_ioctl is ubi_devices_mutex, but the lock held by ubi_cdev_ioctl is ubi->device_mutex. Therefore, the two locks can be concurrent. ctrl_cdev_ioctl contains two operations: ubi_attach and ubi_detach. ubi_detach is bug-free because it uses reference counting to prevent concurrency. However, uif_init and uif_close in ubi_attach may race with ubi_cdev_ioctl. uif_init will race with ubi_cdev_ioctl as in the following stack. cpu1 cpu2 cpu3 _______________________|________________________|______________________ ctrl_cdev_ioctl ubi_attach_mtd_dev uif_init ubi_cdev_ioctl ubi_create_volume cdev_device_add ubi_add_volume // sysfs exist kill_volumes ubi_cdev_ioctl ubi_remove_volume cdev_device_del // first free ubi_free_volume cdev_del // double free cdev_device_del And uif_close will race with ubi_cdev_ioctl as in the following stack. cpu1 cpu2 cpu3 _______________________|________________________|______________________ ctrl_cdev_ioctl ubi_attach_mtd_dev uif_init ubi_cdev_ioctl ubi_create_volume cdev_device_add ubi_debugfs_init_dev //error goto out_uif; uif_close kill_volumes ubi_cdev_ioctl ubi_remove_volume cdev_device_del // first free ubi_free_volume // double free The cause of this problem is that commit 714fb87e8bc0 make device "available" before it becomes accessible via sysfs. Therefore, we roll back the modification. We will fix the race condition between ubi device creation and udev by removing ubi_get_device in vol_attribute_show and dev_attribute_show.This avoids accessing uninitialized ubi_devices[ubi_num]. ubi_get_device is used to prevent devices from being deleted during sysfs execution. However, now kernfs ensures that devices will not be deleted before all reference counting are released. The key process is shown in the following stack. device_del device_remove_attrs device_remove_groups sysfs_remove_groups sysfs_remove_group remove_files kernfs_remove_by_name kernfs_remove_by_name_ns __kernfs_remove kernfs_drain Fixes: 714fb87e8bc0 ("ubi: Fix race condition between ubi device creation and udev") Reported-by: Hulk Robot Signed-off-by: Baokun Li Signed-off-by: Richard Weinberger Signed-off-by: Greg Kroah-Hartman --- drivers/mtd/ubi/build.c | 9 +-------- drivers/mtd/ubi/vmt.c | 8 +------- 2 files changed, 2 insertions(+), 15 deletions(-) --- a/drivers/mtd/ubi/build.c +++ b/drivers/mtd/ubi/build.c @@ -351,9 +351,6 @@ static ssize_t dev_attribute_show(struct * we still can use 'ubi->ubi_num'. */ ubi = container_of(dev, struct ubi_device, dev); - ubi = ubi_get_device(ubi->ubi_num); - if (!ubi) - return -ENODEV; if (attr == &dev_eraseblock_size) ret = sprintf(buf, "%d\n", ubi->leb_size); @@ -382,7 +379,6 @@ static ssize_t dev_attribute_show(struct else ret = -EINVAL; - ubi_put_device(ubi); return ret; } @@ -979,9 +975,6 @@ int ubi_attach_mtd_dev(struct mtd_info * goto out_detach; } - /* Make device "available" before it becomes accessible via sysfs */ - ubi_devices[ubi_num] = ubi; - err = uif_init(ubi); if (err) goto out_detach; @@ -1026,6 +1019,7 @@ int ubi_attach_mtd_dev(struct mtd_info * wake_up_process(ubi->bgt_thread); spin_unlock(&ubi->wl_lock); + ubi_devices[ubi_num] = ubi; ubi_notify_all(ubi, UBI_VOLUME_ADDED, NULL); return ubi_num; @@ -1034,7 +1028,6 @@ out_debugfs: out_uif: uif_close(ubi); out_detach: - ubi_devices[ubi_num] = NULL; ubi_wl_close(ubi); ubi_free_all_volumes(ubi); vfree(ubi->vtbl); --- a/drivers/mtd/ubi/vmt.c +++ b/drivers/mtd/ubi/vmt.c @@ -56,16 +56,11 @@ static ssize_t vol_attribute_show(struct { int ret; struct ubi_volume *vol = container_of(dev, struct ubi_volume, dev); - struct ubi_device *ubi; - - ubi = ubi_get_device(vol->ubi->ubi_num); - if (!ubi) - return -ENODEV; + struct ubi_device *ubi = vol->ubi; spin_lock(&ubi->volumes_lock); if (!ubi->volumes[vol->vol_id]) { spin_unlock(&ubi->volumes_lock); - ubi_put_device(ubi); return -ENODEV; } /* Take a reference to prevent volume removal */ @@ -103,7 +98,6 @@ static ssize_t vol_attribute_show(struct vol->ref_count -= 1; ubi_assert(vol->ref_count >= 0); spin_unlock(&ubi->volumes_lock); - ubi_put_device(ubi); return ret; }