public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Hyunwoo Kim <imv4bel@gmail.com>,
	kernel test robot <lkp@intel.com>,
	Dan Carpenter <error27@gmail.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Sasha Levin <sashal@kernel.org>,
	gregkh@linuxfoundation.org, linma@zju.edu.cn,
	benjamin.tissoires@redhat.com, sre@kernel.org,
	colin.i.king@gmail.com, keitasuzuki.park@sslab.ics.keio.ac.jp,
	linux-media@vger.kernel.org
Subject: [PATCH AUTOSEL 5.15 21/43] media: dvb-core: Fix use-after-free due to race at dvb_register_device()
Date: Thu, 25 May 2023 14:38:32 -0400	[thread overview]
Message-ID: <20230525183854.1855431-21-sashal@kernel.org> (raw)
In-Reply-To: <20230525183854.1855431-1-sashal@kernel.org>

From: Hyunwoo Kim <imv4bel@gmail.com>

[ 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@gmail.com
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 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/dvbdev.c
index 6e2b7e97da176..2ff8a1b776fb4 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -37,6 +37,7 @@
 #include <media/tuner.h>
 
 static DEFINE_MUTEX(dvbdev_mutex);
+static LIST_HEAD(dvbdevfops_list);
 static int dvbdev_debug;
 
 module_param(dvbdev_debug, int, 0644);
@@ -462,14 +463,15 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 			enum dvb_device_type type, int demux_sink_pads)
 {
 	struct dvb_device *dvbdev;
-	struct file_operations *dvbdevfops;
+	struct file_operations *dvbdevfops = NULL;
+	struct dvbdevfops_node *node = NULL, *new_node = NULL;
 	struct device *clsdev;
 	int minor;
 	int id, ret;
 
 	mutex_lock(&dvbdev_register_lock);
 
-	if ((id = dvbdev_get_free_id (adap, type)) < 0){
+	if ((id = dvbdev_get_free_id (adap, type)) < 0) {
 		mutex_unlock(&dvbdev_register_lock);
 		*pdvbdev = NULL;
 		pr_err("%s: couldn't find free device id\n", __func__);
@@ -477,18 +479,45 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 	}
 
 	*pdvbdev = dvbdev = kzalloc(sizeof(*dvbdev), GFP_KERNEL);
-
 	if (!dvbdev){
 		mutex_unlock(&dvbdev_register_lock);
 		return -ENOMEM;
 	}
 
-	dvbdevfops = 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 == adap->module &&
+				node->type == type &&
+				node->template == template) {
+			dvbdevfops = node->fops;
+			break;
+		}
+	}
 
-	if (!dvbdevfops){
-		kfree (dvbdev);
-		mutex_unlock(&dvbdev_register_lock);
-		return -ENOMEM;
+	if (dvbdevfops == NULL) {
+		dvbdevfops = kmemdup(template->fops, sizeof(*dvbdevfops), GFP_KERNEL);
+		if (!dvbdevfops) {
+			kfree(dvbdev);
+			mutex_unlock(&dvbdev_register_lock);
+			return -ENOMEM;
+		}
+
+		new_node = kzalloc(sizeof(struct dvbdevfops_node), GFP_KERNEL);
+		if (!new_node) {
+			kfree(dvbdevfops);
+			kfree(dvbdev);
+			mutex_unlock(&dvbdev_register_lock);
+			return -ENOMEM;
+		}
+
+		new_node->fops = dvbdevfops;
+		new_node->type = type;
+		new_node->template = template;
+		list_add_tail (&new_node->list_head, &dvbdevfops_list);
 	}
 
 	memcpy(dvbdev, template, sizeof(struct dvb_device));
@@ -499,20 +528,20 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 	dvbdev->priv = priv;
 	dvbdev->fops = dvbdevfops;
 	init_waitqueue_head (&dvbdev->wait_queue);
-
 	dvbdevfops->owner = adap->module;
-
 	list_add_tail (&dvbdev->list_head, &adap->device_list);
-
 	down_write(&minor_rwsem);
 #ifdef CONFIG_DVB_DYNAMIC_MINORS
 	for (minor = 0; minor < MAX_DVB_MINORS; minor++)
 		if (dvb_minors[minor] == NULL)
 			break;
-
 	if (minor == 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);
@@ -521,41 +550,47 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 #else
 	minor = nums2minor(adap->num, type, id);
 #endif
-
 	dvbdev->minor = minor;
 	dvb_minors[minor] = dvb_device_get(dvbdev);
 	up_write(&minor_rwsem);
-
 	ret = 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;
 	}
 
-	mutex_unlock(&dvbdev_register_lock);
-
 	clsdev = 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);
 
+	mutex_unlock(&dvbdev_register_lock);
 	return 0;
 }
 EXPORT_SYMBOL(dvb_register_device);
@@ -584,7 +619,6 @@ static void dvb_free_device(struct kref *ref)
 {
 	struct dvb_device *dvbdev = container_of(ref, struct dvb_device, ref);
 
-	kfree (dvbdev->fops);
 	kfree (dvbdev);
 }
 
@@ -1090,9 +1124,17 @@ static int __init init_dvbdev(void)
 
 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);
+	}
 }
 
 subsys_initcall(init_dvbdev);
diff --git a/include/media/dvbdev.h b/include/media/dvbdev.h
index ac60c9fcfe9a6..34b01ebf32828 100644
--- a/include/media/dvbdev.h
+++ b/include/media/dvbdev.h
@@ -189,6 +189,21 @@ struct dvb_device {
 	void *priv;
 };
 
+/**
+ * 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
  *
-- 
2.39.2


  parent reply	other threads:[~2023-05-25 18:47 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-25 18:38 [PATCH AUTOSEL 5.15 01/43] nvme-pci: add NVME_QUIRK_BOGUS_NID for HS-SSD-FUTURE 2048G Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 02/43] nvme-pci: add quirk for missing secondary temperature thresholds Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 03/43] ASoC: dwc: limit the number of overrun messages Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 04/43] selftests/ftrace: Improve integration with kselftest runner Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 05/43] um: harddog: fix modular build Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 06/43] xfrm: Check if_id in inbound policy/secpath match Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 07/43] ASoC: dt-bindings: Adjust #sound-dai-cells on TI's single-DAI codecs Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 08/43] ASoC: ssm2602: Add workaround for playback distortions Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 09/43] media: dvb_demux: fix a bug for the continuity counter Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 10/43] media: dvb-usb: az6027: fix three null-ptr-deref in az6027_i2c_xfer() Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 11/43] media: dvb-usb-v2: ec168: fix null-ptr-deref in ec168_i2c_xfer() Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 12/43] media: dvb-usb-v2: ce6230: fix null-ptr-deref in ce6230_i2c_master_xfer() Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 13/43] media: dvb-usb-v2: rtl28xxu: fix null-ptr-deref in rtl28xxu_i2c_xfer Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 14/43] media: dvb-usb: digitv: fix null-ptr-deref in digitv_i2c_xfer() Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 15/43] media: dvb-usb: dw2102: fix uninit-value in su3000_read_mac_address Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 16/43] media: netup_unidvb: fix irq init by register it at the end of probe Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 17/43] media: dvb_ca_en50221: fix a size write bug Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 18/43] media: ttusb-dec: fix memory leak in ttusb_dec_exit_dvb() Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 19/43] media: mn88443x: fix !CONFIG_OF error by drop of_match_ptr from ID table Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 20/43] media: dvb-core: Fix use-after-free due on race condition at dvb_net Sasha Levin
2023-05-25 18:38 ` Sasha Levin [this message]
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 22/43] media: dvb-core: Fix kernel WARNING for blocking operation in wait_event*() Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 23/43] media: dvb-core: Fix use-after-free due to race condition at dvb_ca_en50221 Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 24/43] tipc: add tipc_bearer_min_mtu to calculate min mtu Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 25/43] s390/pkey: zeroize key blobs Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 26/43] s390/topology: honour nr_cpu_ids when adding CPUs Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 27/43] ACPI: resource: Add IRQ override quirk for LG UltraPC 17U70P Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 28/43] wifi: rtl8xxxu: fix authentication timeout due to incorrect RCR value Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 29/43] ARM: dts: stm32: add pin map for CAN controller on stm32f7 Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 30/43] ARM: dts: stm32: add CAN support on stm32f746 Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 31/43] arm64/mm: mark private VM_FAULT_X defines as vm_fault_t Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 32/43] arm64: vdso: Pass (void *) to virt_to_page() Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 33/43] wifi: mac80211: simplify chanctx allocation Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 34/43] scsi: core: Decrease scsi_device's iorequest_cnt if dispatch failed Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 35/43] wifi: b43: fix incorrect __packed annotation Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 36/43] netfilter: conntrack: define variables exp_nat_nla_policy and any_addr with CONFIG_NF_NAT Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 37/43] nvme-multipath: don't call blk_mark_disk_dead in nvme_mpath_remove_disk Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 38/43] ALSA: oss: avoid missing-prototype warnings Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 39/43] drm/msm: Be more shouty if per-process pgtables aren't working Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 40/43] atm: hide unused procfs functions Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 41/43] mdio_bus: unhide mdio_bus_init prototype Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 42/43] drm/amdgpu: skip disabling fence driver src_irqs when device is unplugged Sasha Levin
2023-05-25 18:38 ` [PATCH AUTOSEL 5.15 43/43] nvme-pci: Add quirk for Teamgroup MP33 SSD Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230525183854.1855431-21-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=benjamin.tissoires@redhat.com \
    --cc=colin.i.king@gmail.com \
    --cc=error27@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=imv4bel@gmail.com \
    --cc=keitasuzuki.park@sslab.ics.keio.ac.jp \
    --cc=linma@zju.edu.cn \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=mchehab@kernel.org \
    --cc=sre@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox