Commit 7704e6be authored by Alexander Usyskin's avatar Alexander Usyskin Committed by Greg Kroah-Hartman
Browse files

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: default avatarAlexander Usyskin <alexander.usyskin@intel.com>
Link: https://lore.kernel.org/r/20250826125617.1166546-1-alexander.usyskin@intel.com


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 76254bc4
Loading
Loading
Loading
Loading
+3 −3
Original line number Diff line number Diff line
@@ -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:
+14 −10
Original line number Diff line number Diff line
@@ -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)
+41 −41
Original line number Diff line number Diff line
@@ -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,9 +981,9 @@ 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;
	}
@@ -991,8 +991,8 @@ int mei_cl_disconnect(struct mei_cl *cl)
	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;
+3 −3
Original line number Diff line number Diff line
@@ -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_ */
+4 −4
Original line number Diff line number Diff line
@@ -30,7 +30,7 @@ static int mei_dmam_dscr_alloc(struct mei_device *dev,
	if (dscr->vaddr)
		return 0;

	dscr->vaddr = dmam_alloc_coherent(dev->dev, dscr->size, &dscr->daddr,
	dscr->vaddr = dmam_alloc_coherent(dev->parent, dscr->size, &dscr->daddr,
					  GFP_KERNEL);
	if (!dscr->vaddr)
		return -ENOMEM;
@@ -50,7 +50,7 @@ static void mei_dmam_dscr_free(struct mei_device *dev,
	if (!dscr->vaddr)
		return;

	dmam_free_coherent(dev->dev, dscr->size, dscr->vaddr, dscr->daddr);
	dmam_free_coherent(dev->parent, dscr->size, dscr->vaddr, dscr->daddr);
	dscr->vaddr = NULL;
}

@@ -177,7 +177,7 @@ void mei_dma_ring_read(struct mei_device *dev, unsigned char *buf, u32 len)
	if (WARN_ON(!ctrl))
		return;

	dev_dbg(dev->dev, "reading from dma %u bytes\n", len);
	dev_dbg(&dev->dev, "reading from dma %u bytes\n", len);

	if (!len)
		return;
@@ -254,7 +254,7 @@ void mei_dma_ring_write(struct mei_device *dev, unsigned char *buf, u32 len)
	if (WARN_ON(!ctrl))
		return;

	dev_dbg(dev->dev, "writing to dma %u bytes\n", len);
	dev_dbg(&dev->dev, "writing to dma %u bytes\n", len);
	hbuf_depth = mei_dma_ring_hbuf_depth(dev);
	wr_idx = READ_ONCE(ctrl->hbuf_wr_idx) & (hbuf_depth - 1);
	slots = mei_data2slots(len);
Loading