From nobody Fri Dec 19 20:14:45 2025 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 3278E29617D; Sun, 24 Mar 2024 23:51:55 +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=1711324315; cv=none; b=OWQgZxaXOvrDr5+DYB98wdu1YNAKs6fGuIwQz+FnQz6BOPGTb7d7dnLWXTU5ApGYIvYy4Deyl9oB2ei5FXbgOr+VkGgpOmyVbW5OLy7WnkNJY6nr4hV0joY+XE1qThGFcAJ9d10eKeUlowZz0oBPI7rtnQUt2/nBYxKybpGc82M= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1711324315; c=relaxed/simple; bh=deSVadvuUG1ckQLc/0sZcVTEfikkjUcgOGef72yAj6Y=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=hOS7SlVRx8G+lfGhWb5P4+CalOzauGrgR8+T3B5WsW7LHYLvtH407ZcPF5jf3XlaZM9A/Gp7TB2+hXhk88ohbgDT1HprpQ6EI+T+1menDFCMBjjqbaMCpOtDCZmQCXWeOpXSkXE3vBvfXekdvs4xf6+PLF/odNA854bQY/4vKSc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=jLsFVE2s; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="jLsFVE2s" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1AB6CC43394; Sun, 24 Mar 2024 23:51:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1711324315; bh=deSVadvuUG1ckQLc/0sZcVTEfikkjUcgOGef72yAj6Y=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=jLsFVE2sOhNK7rRz0hBk55Rgg79z5hnTh+9iMYkpd1XRsy6+uq9qvuVZA3hu/D2ic IFkquaoADyjtcRqf/WQDvtdIrJA02/32vi1t4ARxo5jXG2tWjHYG7uIF/Gu5njR32Z U5TgLzY+7cl6rcFPFMljhxbW9spj6CIoX/LSLVwHqIOX5+FfFGHG1DYDPbvzGXzB51 0vuX1CXl/QEiTgqJY3uHJF+nvUyyOkmAJdhbdG+56/FXsy2FXNgQ7CYmhTT4WptcG4 dRwX/adxp8ytjeYDzzdCVtS/67gTYA+mVPTWzo2RqKiPcMrk/iWmyRnCrUWorI2e5z 8tHcRSI5Z+rsA== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Hyunwoo Kim , kernel test robot , Dan Carpenter , Mauro Carvalho Chehab , Sasha Levin Subject: [PATCH 4.19 086/148] media: dvb-core: Fix use-after-free due to race at dvb_register_device() Date: Sun, 24 Mar 2024 19:49:10 -0400 Message-ID: <20240324235012.1356413-87-sashal@kernel.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240324235012.1356413-1-sashal@kernel.org> References: <20240324235012.1356413-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Hyunwoo Kim [ Upstream commit 627bb528b086b4136315c25d6a447a98ea9448d3 ] dvb_register_device() dynamically allocates fops with kmemdup() to set the fops->owner. And these fops are registered in 'file->f_ops' using replace_fops() in the dvb_device_open() process, and kfree()d in dvb_free_device(). However, it is not common to use dynamically allocated fops instead of 'static const' fops as an argument of replace_fops(), and UAF may occur. These UAFs can occur on any dvb type using dvb_register_device(), such as dvb_dvr, dvb_demux, dvb_frontend, dvb_net, etc. So, instead of kfree() the fops dynamically allocated in dvb_register_device() in dvb_free_device() called during the .disconnect() process, kfree() it collectively in exit_dvbdev() called when the dvbdev.c module is removed. Link: https://lore.kernel.org/linux-media/20221117045925.14297-4-imv4bel@gm= ail.com Signed-off-by: Hyunwoo Kim Reported-by: kernel test robot Reported-by: Dan Carpenter Signed-off-by: Mauro Carvalho Chehab Stable-dep-of: 8c64f4cdf4e6 ("media: edia: dvbdev: fix a use-after-free") Signed-off-by: Sasha Levin --- drivers/media/dvb-core/dvbdev.c | 84 ++++++++++++++++++++++++--------- include/media/dvbdev.h | 15 ++++++ 2 files changed, 78 insertions(+), 21 deletions(-) diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbde= v.c index cf0c428f5776b..35c272d77375e 100644 --- a/drivers/media/dvb-core/dvbdev.c +++ b/drivers/media/dvb-core/dvbdev.c @@ -37,6 +37,7 @@ #include =20 static DEFINE_MUTEX(dvbdev_mutex); +static LIST_HEAD(dvbdevfops_list); static int dvbdev_debug; =20 module_param(dvbdev_debug, int, 0644); @@ -464,14 +465,15 @@ int dvb_register_device(struct dvb_adapter *adap, str= uct dvb_device **pdvbdev, enum dvb_device_type type, int demux_sink_pads) { struct dvb_device *dvbdev; - struct file_operations *dvbdevfops; + struct file_operations *dvbdevfops =3D NULL; + struct dvbdevfops_node *node =3D NULL, *new_node =3D NULL; struct device *clsdev; int minor; int id, ret; =20 mutex_lock(&dvbdev_register_lock); =20 - if ((id =3D dvbdev_get_free_id (adap, type)) < 0){ + if ((id =3D dvbdev_get_free_id (adap, type)) < 0) { mutex_unlock(&dvbdev_register_lock); *pdvbdev =3D NULL; pr_err("%s: couldn't find free device id\n", __func__); @@ -479,18 +481,45 @@ int dvb_register_device(struct dvb_adapter *adap, str= uct dvb_device **pdvbdev, } =20 *pdvbdev =3D dvbdev =3D kzalloc(sizeof(*dvbdev), GFP_KERNEL); - if (!dvbdev){ mutex_unlock(&dvbdev_register_lock); return -ENOMEM; } =20 - dvbdevfops =3D kmemdup(template->fops, sizeof(*dvbdevfops), GFP_KERNEL); + /* + * When a device of the same type is probe()d more than once, + * the first allocated fops are used. This prevents memory leaks + * that can occur when the same device is probe()d repeatedly. + */ + list_for_each_entry(node, &dvbdevfops_list, list_head) { + if (node->fops->owner =3D=3D adap->module && + node->type =3D=3D type && + node->template =3D=3D template) { + dvbdevfops =3D node->fops; + break; + } + } =20 - if (!dvbdevfops){ - kfree (dvbdev); - mutex_unlock(&dvbdev_register_lock); - return -ENOMEM; + if (dvbdevfops =3D=3D NULL) { + dvbdevfops =3D kmemdup(template->fops, sizeof(*dvbdevfops), GFP_KERNEL); + if (!dvbdevfops) { + kfree(dvbdev); + mutex_unlock(&dvbdev_register_lock); + return -ENOMEM; + } + + new_node =3D kzalloc(sizeof(struct dvbdevfops_node), GFP_KERNEL); + if (!new_node) { + kfree(dvbdevfops); + kfree(dvbdev); + mutex_unlock(&dvbdev_register_lock); + return -ENOMEM; + } + + new_node->fops =3D dvbdevfops; + new_node->type =3D type; + new_node->template =3D template; + list_add_tail (&new_node->list_head, &dvbdevfops_list); } =20 memcpy(dvbdev, template, sizeof(struct dvb_device)); @@ -501,20 +530,20 @@ int dvb_register_device(struct dvb_adapter *adap, str= uct dvb_device **pdvbdev, dvbdev->priv =3D priv; dvbdev->fops =3D dvbdevfops; init_waitqueue_head (&dvbdev->wait_queue); - dvbdevfops->owner =3D adap->module; - list_add_tail (&dvbdev->list_head, &adap->device_list); - down_write(&minor_rwsem); #ifdef CONFIG_DVB_DYNAMIC_MINORS for (minor =3D 0; minor < MAX_DVB_MINORS; minor++) if (dvb_minors[minor] =3D=3D NULL) break; - if (minor =3D=3D MAX_DVB_MINORS) { + if (new_node) { + list_del (&new_node->list_head); + kfree(dvbdevfops); + kfree(new_node); + } list_del (&dvbdev->list_head); - kfree(dvbdevfops); kfree(dvbdev); up_write(&minor_rwsem); mutex_unlock(&dvbdev_register_lock); @@ -523,41 +552,47 @@ int dvb_register_device(struct dvb_adapter *adap, str= uct dvb_device **pdvbdev, #else minor =3D nums2minor(adap->num, type, id); #endif - dvbdev->minor =3D minor; dvb_minors[minor] =3D dvb_device_get(dvbdev); up_write(&minor_rwsem); - ret =3D dvb_register_media_device(dvbdev, type, minor, demux_sink_pads); if (ret) { pr_err("%s: dvb_register_media_device failed to create the mediagraph\n", __func__); - + if (new_node) { + list_del (&new_node->list_head); + kfree(dvbdevfops); + kfree(new_node); + } dvb_media_device_free(dvbdev); list_del (&dvbdev->list_head); - kfree(dvbdevfops); kfree(dvbdev); mutex_unlock(&dvbdev_register_lock); return ret; } =20 - mutex_unlock(&dvbdev_register_lock); - clsdev =3D device_create(dvb_class, adap->device, MKDEV(DVB_MAJOR, minor), dvbdev, "dvb%d.%s%d", adap->num, dnames[type], id); if (IS_ERR(clsdev)) { pr_err("%s: failed to create device dvb%d.%s%d (%ld)\n", __func__, adap->num, dnames[type], id, PTR_ERR(clsdev)); + if (new_node) { + list_del (&new_node->list_head); + kfree(dvbdevfops); + kfree(new_node); + } dvb_media_device_free(dvbdev); list_del (&dvbdev->list_head); - kfree(dvbdevfops); kfree(dvbdev); + mutex_unlock(&dvbdev_register_lock); return PTR_ERR(clsdev); } + dprintk("DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n", adap->num, dnames[type], id, minor, minor); =20 + mutex_unlock(&dvbdev_register_lock); return 0; } EXPORT_SYMBOL(dvb_register_device); @@ -586,7 +621,6 @@ static void dvb_free_device(struct kref *ref) { struct dvb_device *dvbdev =3D container_of(ref, struct dvb_device, ref); =20 - kfree (dvbdev->fops); kfree (dvbdev); } =20 @@ -1082,9 +1116,17 @@ static int __init init_dvbdev(void) =20 static void __exit exit_dvbdev(void) { + struct dvbdevfops_node *node, *next; + class_destroy(dvb_class); cdev_del(&dvb_device_cdev); unregister_chrdev_region(MKDEV(DVB_MAJOR, 0), MAX_DVB_MINORS); + + list_for_each_entry_safe(node, next, &dvbdevfops_list, list_head) { + list_del (&node->list_head); + kfree(node->fops); + kfree(node); + } } =20 subsys_initcall(init_dvbdev); diff --git a/include/media/dvbdev.h b/include/media/dvbdev.h index 09279ed0051ea..0e2bda5ccadd5 100644 --- a/include/media/dvbdev.h +++ b/include/media/dvbdev.h @@ -189,6 +189,21 @@ struct dvb_device { void *priv; }; =20 +/** + * struct dvbdevfops_node - fops nodes registered in dvbdevfops_list + * + * @fops: Dynamically allocated fops for ->owner registration + * @type: type of dvb_device + * @template: dvb_device used for registration + * @list_head: list_head for dvbdevfops_list + */ +struct dvbdevfops_node { + struct file_operations *fops; + enum dvb_device_type type; + const struct dvb_device *template; + struct list_head list_head; +}; + /** * dvb_device_get - Increase dvb_device reference * --=20 2.43.0