mei: hook mei_device on class device

mei_device lifetime was managed by devm procedure of parent device.
But such memory is freed on device_del.
Mei_device object is used by client object that may be alive after
parent device is removed.
It may lead to use-after-free if discrete graphics driver unloads
mei_gsc auxiliary device while user-space holds open handle to mei
character device.

Connect mei_device structure lifteme to mei class device lifetime
by adding mei_device free to class device remove callback.

Move exising parent device pointer to separate field in mei_device
to avoid misuse.

Allocate character device dynamically and allow to control its own
lifetime as it may outlive mei_device structure while character
device closes after parent device is removed from the system.

Leave power management on parent device as we overwrite pci runtime
pm procedure and user-space is expecting it there.

Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14201
Signed-off-by: Alexander Usyskin <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

Author: ausyskin

drivers/misc/mei/bus-fixup.c

@@ -386,7 +386,7 @@ static int mei_nfc_if_version(struct mei_cl *cl,
 	ret = __mei_cl_send(cl, (u8 *)&cmd, sizeof(cmd), 0,
 			    MEI_CL_IO_TX_BLOCKING);
 	if (ret < 0) {
-		dev_err(bus->dev, "Could not send IF version cmd ret = %d\n", ret);
+		dev_err(&bus->dev, "Could not send IF version cmd ret = %d\n", ret);
 		return ret;
 	}
 
@@ -401,14 +401,14 @@ static int mei_nfc_if_version(struct mei_cl *cl,
 	bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length, &vtag,
 				   0, 0);
 	if (bytes_recv < 0 || (size_t)bytes_recv < if_version_length) {
-		dev_err(bus->dev, "Could not read IF version ret = %d\n", bytes_recv);
+		dev_err(&bus->dev, "Could not read IF version ret = %d\n", bytes_recv);
 		ret = -EIO;
 		goto err;
 	}
 
 	memcpy(ver, reply->data, sizeof(*ver));
 
-	dev_info(bus->dev, "NFC MEI VERSION: IVN 0x%x Vendor ID 0x%x Type 0x%x\n",
+	dev_info(&bus->dev, "NFC MEI VERSION: IVN 0x%x Vendor ID 0x%x Type 0x%x\n",
 		 ver->fw_ivn, ver->vendor_id, ver->radio_type);
 
 err:

drivers/misc/mei/bus.c

@@ -637,7 +637,7 @@ EXPORT_SYMBOL_GPL(mei_cldev_enabled);
  */
 static bool mei_cl_bus_module_get(struct mei_cl_device *cldev)
 {
-	return try_module_get(cldev->bus->dev->driver->owner);
+	return try_module_get(cldev->bus->parent->driver->owner);
 }
 
 /**
@@ -647,7 +647,7 @@ static bool mei_cl_bus_module_get(struct mei_cl_device *cldev)
  */
 static void mei_cl_bus_module_put(struct mei_cl_device *cldev)
 {
-	module_put(cldev->bus->dev->driver->owner);
+	module_put(cldev->bus->parent->driver->owner);
 }
 
 /**
@@ -1285,16 +1285,20 @@ static const struct bus_type mei_cl_bus_type = {
 
 static struct mei_device *mei_dev_bus_get(struct mei_device *bus)
 {
-	if (bus)
-		get_device(bus->dev);
+	if (bus) {
+		get_device(&bus->dev);
+		get_device(bus->parent);
+	}
 
 	return bus;
 }
 
 static void mei_dev_bus_put(struct mei_device *bus)
 {
-	if (bus)
-		put_device(bus->dev);
+	if (bus) {
+		put_device(bus->parent);
+		put_device(&bus->dev);
+	}
 }
 
 static void mei_cl_bus_dev_release(struct device *dev)
@@ -1328,7 +1332,7 @@ static const struct device_type mei_cl_device_type = {
 static inline void mei_cl_bus_set_name(struct mei_cl_device *cldev)
 {
 	dev_set_name(&cldev->dev, "%s-%pUl",
-		     dev_name(cldev->bus->dev),
+		     dev_name(cldev->bus->parent),
 		     mei_me_cl_uuid(cldev->me_cl));
 }
 
@@ -1357,7 +1361,7 @@ static struct mei_cl_device *mei_cl_bus_dev_alloc(struct mei_device *bus,
 	}
 
 	device_initialize(&cldev->dev);
-	cldev->dev.parent = bus->dev;
+	cldev->dev.parent = bus->parent;
 	cldev->dev.bus    = &mei_cl_bus_type;
 	cldev->dev.type   = &mei_cl_device_type;
 	cldev->bus        = mei_dev_bus_get(bus);
@@ -1492,7 +1496,7 @@ static void mei_cl_bus_dev_init(struct mei_device *bus,
 
 	WARN_ON(!mutex_is_locked(&bus->cl_bus_lock));
 
-	dev_dbg(bus->dev, "initializing %pUl", mei_me_cl_uuid(me_cl));
+	dev_dbg(&bus->dev, "initializing %pUl", mei_me_cl_uuid(me_cl));
 
 	if (me_cl->bus_added)
 		return;
@@ -1543,7 +1547,7 @@ static void mei_cl_bus_rescan(struct mei_device *bus)
 	}
 	mutex_unlock(&bus->cl_bus_lock);
 
-	dev_dbg(bus->dev, "rescan end");
+	dev_dbg(&bus->dev, "rescan end");
 }
 
 void mei_cl_bus_rescan_work(struct work_struct *work)

drivers/misc/mei/client.c

@@ -262,7 +262,7 @@ void mei_me_cl_rm_by_uuid(struct mei_device *dev, const uuid_le *uuid)
 {
 	struct mei_me_client *me_cl;
 
-	dev_dbg(dev->dev, "remove %pUl\n", uuid);
+	dev_dbg(&dev->dev, "remove %pUl\n", uuid);
 
 	down_write(&dev->me_clients_rwsem);
 	me_cl = __mei_me_cl_by_uuid(dev, uuid);
@@ -635,12 +635,12 @@ int mei_cl_link(struct mei_cl *cl)
 
 	id = find_first_zero_bit(dev->host_clients_map, MEI_CLIENTS_MAX);
 	if (id >= MEI_CLIENTS_MAX) {
-		dev_err(dev->dev, "id exceeded %d", MEI_CLIENTS_MAX);
+		dev_err(&dev->dev, "id exceeded %d", MEI_CLIENTS_MAX);
 		return -EMFILE;
 	}
 
 	if (dev->open_handle_count >= MEI_MAX_OPEN_HANDLE_COUNT) {
-		dev_err(dev->dev, "open_handle_count exceeded %d",
+		dev_err(&dev->dev, "open_handle_count exceeded %d",
 			MEI_MAX_OPEN_HANDLE_COUNT);
 		return -EMFILE;
 	}
@@ -709,9 +709,9 @@ void mei_host_client_init(struct mei_device *dev)
 
 	schedule_work(&dev->bus_rescan_work);
 
-	pm_runtime_mark_last_busy(dev->dev);
-	dev_dbg(dev->dev, "rpm: autosuspend\n");
-	pm_request_autosuspend(dev->dev);
+	pm_runtime_mark_last_busy(dev->parent);
+	dev_dbg(&dev->dev, "rpm: autosuspend\n");
+	pm_request_autosuspend(dev->parent);
 }
 
 /**
@@ -724,12 +724,12 @@ bool mei_hbuf_acquire(struct mei_device *dev)
 {
 	if (mei_pg_state(dev) == MEI_PG_ON ||
 	    mei_pg_in_transition(dev)) {
-		dev_dbg(dev->dev, "device is in pg\n");
+		dev_dbg(&dev->dev, "device is in pg\n");
 		return false;
 	}
 
 	if (!dev->hbuf_is_ready) {
-		dev_dbg(dev->dev, "hbuf is not ready\n");
+		dev_dbg(&dev->dev, "hbuf is not ready\n");
 		return false;
 	}
 
@@ -981,18 +981,18 @@ int mei_cl_disconnect(struct mei_cl *cl)
 		return 0;
 	}
 
-	rets = pm_runtime_get(dev->dev);
+	rets = pm_runtime_get(dev->parent);
 	if (rets < 0 && rets != -EINPROGRESS) {
-		pm_runtime_put_noidle(dev->dev);
+		pm_runtime_put_noidle(dev->parent);
 		cl_err(dev, cl, "rpm: get failed %d\n", rets);
 		return rets;
 	}
 
 	rets = __mei_cl_disconnect(cl);
 
 	cl_dbg(dev, cl, "rpm: autosuspend\n");
-	pm_runtime_mark_last_busy(dev->dev);
-	pm_runtime_put_autosuspend(dev->dev);
+	pm_runtime_mark_last_busy(dev->parent);
+	pm_runtime_put_autosuspend(dev->parent);
 
 	return rets;
 }
@@ -1118,9 +1118,9 @@ int mei_cl_connect(struct mei_cl *cl, struct mei_me_client *me_cl,
 		goto nortpm;
 	}
 
-	rets = pm_runtime_get(dev->dev);
+	rets = pm_runtime_get(dev->parent);
 	if (rets < 0 && rets != -EINPROGRESS) {
-		pm_runtime_put_noidle(dev->dev);
+		pm_runtime_put_noidle(dev->parent);
 		cl_err(dev, cl, "rpm: get failed %d\n", rets);
 		goto nortpm;
 	}
@@ -1167,8 +1167,8 @@ int mei_cl_connect(struct mei_cl *cl, struct mei_me_client *me_cl,
 	rets = cl->status;
 out:
 	cl_dbg(dev, cl, "rpm: autosuspend\n");
-	pm_runtime_mark_last_busy(dev->dev);
-	pm_runtime_put_autosuspend(dev->dev);
+	pm_runtime_mark_last_busy(dev->parent);
+	pm_runtime_put_autosuspend(dev->parent);
 
 	mei_io_cb_free(cb);
 
@@ -1517,9 +1517,9 @@ int mei_cl_notify_request(struct mei_cl *cl,
 	if (!mei_cl_is_connected(cl))
 		return -ENODEV;
 
-	rets = pm_runtime_get(dev->dev);
+	rets = pm_runtime_get(dev->parent);
 	if (rets < 0 && rets != -EINPROGRESS) {
-		pm_runtime_put_noidle(dev->dev);
+		pm_runtime_put_noidle(dev->parent);
 		cl_err(dev, cl, "rpm: get failed %d\n", rets);
 		return rets;
 	}
@@ -1554,8 +1554,8 @@ int mei_cl_notify_request(struct mei_cl *cl,
 
 out:
 	cl_dbg(dev, cl, "rpm: autosuspend\n");
-	pm_runtime_mark_last_busy(dev->dev);
-	pm_runtime_put_autosuspend(dev->dev);
+	pm_runtime_mark_last_busy(dev->parent);
+	pm_runtime_put_autosuspend(dev->parent);
 
 	mei_io_cb_free(cb);
 	return rets;
@@ -1683,9 +1683,9 @@ int mei_cl_read_start(struct mei_cl *cl, size_t length, const struct file *fp)
 
 	mei_cl_set_read_by_fp(cl, fp);
 
-	rets = pm_runtime_get(dev->dev);
+	rets = pm_runtime_get(dev->parent);
 	if (rets < 0 && rets != -EINPROGRESS) {
-		pm_runtime_put_noidle(dev->dev);
+		pm_runtime_put_noidle(dev->parent);
 		cl_err(dev, cl, "rpm: get failed %d\n", rets);
 		goto nortpm;
 	}
@@ -1702,8 +1702,8 @@ int mei_cl_read_start(struct mei_cl *cl, size_t length, const struct file *fp)
 
 out:
 	cl_dbg(dev, cl, "rpm: autosuspend\n");
-	pm_runtime_mark_last_busy(dev->dev);
-	pm_runtime_put_autosuspend(dev->dev);
+	pm_runtime_mark_last_busy(dev->parent);
+	pm_runtime_put_autosuspend(dev->parent);
 nortpm:
 	if (rets)
 		mei_io_cb_free(cb);
@@ -1972,9 +1972,9 @@ ssize_t mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb, unsigned long time
 	blocking = cb->blocking;
 	data = buf->data;
 
-	rets = pm_runtime_get(dev->dev);
+	rets = pm_runtime_get(dev->parent);
 	if (rets < 0 && rets != -EINPROGRESS) {
-		pm_runtime_put_noidle(dev->dev);
+		pm_runtime_put_noidle(dev->parent);
 		cl_err(dev, cl, "rpm: get failed %zd\n", rets);
 		goto free;
 	}
@@ -2092,8 +2092,8 @@ ssize_t mei_cl_write(struct mei_cl *cl, struct mei_cl_cb *cb, unsigned long time
 	rets = buf_len;
 err:
 	cl_dbg(dev, cl, "rpm: autosuspend\n");
-	pm_runtime_mark_last_busy(dev->dev);
-	pm_runtime_put_autosuspend(dev->dev);
+	pm_runtime_mark_last_busy(dev->parent);
+	pm_runtime_put_autosuspend(dev->parent);
 free:
 	mei_io_cb_free(cb);
 
@@ -2119,8 +2119,8 @@ void mei_cl_complete(struct mei_cl *cl, struct mei_cl_cb *cb)
 		if (waitqueue_active(&cl->tx_wait)) {
 			wake_up_interruptible(&cl->tx_wait);
 		} else {
-			pm_runtime_mark_last_busy(dev->dev);
-			pm_request_autosuspend(dev->dev);
+			pm_runtime_mark_last_busy(dev->parent);
+			pm_request_autosuspend(dev->parent);
 		}
 		break;
 
@@ -2251,7 +2251,7 @@ int mei_cl_irq_dma_unmap(struct mei_cl *cl, struct mei_cl_cb *cb,
 
 static int mei_cl_dma_alloc(struct mei_cl *cl, u8 buf_id, size_t size)
 {
-	cl->dma.vaddr = dmam_alloc_coherent(cl->dev->dev, size,
+	cl->dma.vaddr = dmam_alloc_coherent(&cl->dev->dev, size,
 					    &cl->dma.daddr, GFP_KERNEL);
 	if (!cl->dma.vaddr)
 		return -ENOMEM;
@@ -2265,7 +2265,7 @@ static int mei_cl_dma_alloc(struct mei_cl *cl, u8 buf_id, size_t size)
 static void mei_cl_dma_free(struct mei_cl *cl)
 {
 	cl->dma.buffer_id = 0;
-	dmam_free_coherent(cl->dev->dev,
+	dmam_free_coherent(&cl->dev->dev,
 			   cl->dma.size, cl->dma.vaddr, cl->dma.daddr);
 	cl->dma.size = 0;
 	cl->dma.vaddr = NULL;
@@ -2321,16 +2321,16 @@ int mei_cl_dma_alloc_and_map(struct mei_cl *cl, const struct file *fp,
 		return -EPROTO;
 	}
 
-	rets = pm_runtime_get(dev->dev);
+	rets = pm_runtime_get(dev->parent);
 	if (rets < 0 && rets != -EINPROGRESS) {
-		pm_runtime_put_noidle(dev->dev);
+		pm_runtime_put_noidle(dev->parent);
 		cl_err(dev, cl, "rpm: get failed %d\n", rets);
 		return rets;
 	}
 
 	rets = mei_cl_dma_alloc(cl, buffer_id, size);
 	if (rets) {
-		pm_runtime_put_noidle(dev->dev);
+		pm_runtime_put_noidle(dev->parent);
 		return rets;
 	}
 
@@ -2366,8 +2366,8 @@ int mei_cl_dma_alloc_and_map(struct mei_cl *cl, const struct file *fp,
 		mei_cl_dma_free(cl);
 
 	cl_dbg(dev, cl, "rpm: autosuspend\n");
-	pm_runtime_mark_last_busy(dev->dev);
-	pm_runtime_put_autosuspend(dev->dev);
+	pm_runtime_mark_last_busy(dev->parent);
+	pm_runtime_put_autosuspend(dev->parent);
 
 	mei_io_cb_free(cb);
 	return rets;
@@ -2406,9 +2406,9 @@ int mei_cl_dma_unmap(struct mei_cl *cl, const struct file *fp)
 	if (!cl->dma_mapped)
 		return -EPROTO;
 
-	rets = pm_runtime_get(dev->dev);
+	rets = pm_runtime_get(dev->parent);
 	if (rets < 0 && rets != -EINPROGRESS) {
-		pm_runtime_put_noidle(dev->dev);
+		pm_runtime_put_noidle(dev->parent);
 		cl_err(dev, cl, "rpm: get failed %d\n", rets);
 		return rets;
 	}
@@ -2444,8 +2444,8 @@ int mei_cl_dma_unmap(struct mei_cl *cl, const struct file *fp)
 		mei_cl_dma_free(cl);
 out:
 	cl_dbg(dev, cl, "rpm: autosuspend\n");
-	pm_runtime_mark_last_busy(dev->dev);
-	pm_runtime_put_autosuspend(dev->dev);
+	pm_runtime_mark_last_busy(dev->parent);
+	pm_runtime_put_autosuspend(dev->parent);
 
 	mei_io_cb_free(cb);
 	return rets;

drivers/misc/mei/client.h

@@ -275,12 +275,12 @@ int mei_cl_dma_unmap(struct mei_cl *cl, const struct file *fp);
 #define MEI_CL_PRM(cl) (cl)->host_client_id, mei_cl_me_id(cl)
 
 #define cl_dbg(dev, cl, format, arg...) \
-	dev_dbg((dev)->dev, MEI_CL_FMT format, MEI_CL_PRM(cl), ##arg)
+	dev_dbg(&(dev)->dev, MEI_CL_FMT format, MEI_CL_PRM(cl), ##arg)
 
 #define cl_warn(dev, cl, format, arg...) \
-	dev_warn((dev)->dev, MEI_CL_FMT format, MEI_CL_PRM(cl), ##arg)
+	dev_warn(&(dev)->dev, MEI_CL_FMT format, MEI_CL_PRM(cl), ##arg)
 
 #define cl_err(dev, cl, format, arg...) \
-	dev_err((dev)->dev, MEI_CL_FMT format, MEI_CL_PRM(cl), ##arg)
+	dev_err(&(dev)->dev, MEI_CL_FMT format, MEI_CL_PRM(cl), ##arg)
 
 #endif /* _MEI_CLIENT_H_ */