Commit 9cc238c7 authored by Dan Williams's avatar Dan Williams
Browse files

cxl/pci: Introduce cdevm_file_operations 



In preparation for moving cxl_memdev allocation to the core, introduce
cdevm_file_operations to coordinate file operations shutdown relative to
driver data release.

The motivation for moving cxl_memdev allocation to the core (beyond
better file organization of sysfs attributes in core/ and drivers in
cxl/), is that device lifetime is longer than module lifetime. The cxl_pci
module should be free to come and go without needing to coordinate with
devices that need the text associated with cxl_memdev_release() to stay
resident. The move will fix a use after free bug when looping driver
load / unload with CONFIG_DEBUG_KOBJECT_RELEASE=y.

Another motivation for passing in file_operations to the core cxl_memdev
creation flow is to allow for alternate drivers, like unit test code, to
define their own ioctl backends.

Signed-off-by: default avatarBen Widawsky <ben.widawsky@intel.com>
Reviewed-by: default avatarJonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://lore.kernel.org/r/162792539962.368511.2962268954245340288.stgit@dwillia2-desk3.amr.corp.intel.com


Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
parent 0f06157e

drivers/cxl/cxlmem.h

+15 −0
Original line number Diff line number Diff line
@@ -34,6 +34,21 @@ 
 */

#define CXL_MEM_MAX_DEVS 65536



/**

 * struct cdevm_file_operations - devm coordinated cdev file operations

 * @fops: file operations that are synchronized against @shutdown

 * @shutdown: disconnect driver data

 *

 * @shutdown is invoked in the devres release path to disconnect any

 * driver instance data from @dev. It assumes synchronization with any

 * fops operation that requires driver data. After @shutdown an

 * operation may only reference @device data.

 */

struct cdevm_file_operations {

	struct file_operations fops;

	void (*shutdown)(struct device *dev);

};



/**

 * struct cxl_memdev - CXL bus object representing a Type-3 Memory Device

 * @dev: driver core device object

drivers/cxl/pci.c

+38 −27
Original line number Diff line number Diff line
@@ -806,13 +806,30 @@ static int cxl_memdev_release_file(struct inode *inode, struct file *file) 
	return 0;

}



static const struct file_operations cxl_memdev_fops = {

static struct cxl_memdev *to_cxl_memdev(struct device *dev)

{

	return container_of(dev, struct cxl_memdev, dev);

}



static void cxl_memdev_shutdown(struct device *dev)

{

	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);



	down_write(&cxl_memdev_rwsem);

	cxlmd->cxlm = NULL;

	up_write(&cxl_memdev_rwsem);

}



static const struct cdevm_file_operations cxl_memdev_fops = {

	.fops = {

		.owner = THIS_MODULE,

		.unlocked_ioctl = cxl_memdev_ioctl,

		.open = cxl_memdev_open,

		.release = cxl_memdev_release_file,

		.compat_ioctl = compat_ptr_ioctl,

		.llseek = noop_llseek,

	},

	.shutdown = cxl_memdev_shutdown,

};



static inline struct cxl_mem_command *cxl_mem_find_command(u16 opcode)
@@ -1161,11 +1178,6 @@ static int cxl_mem_setup_regs(struct cxl_mem *cxlm) 
	return ret;

}



static struct cxl_memdev *to_cxl_memdev(struct device *dev)

{

	return container_of(dev, struct cxl_memdev, dev);

}



static void cxl_memdev_release(struct device *dev)

{

	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
@@ -1281,24 +1293,22 @@ static const struct device_type cxl_memdev_type = { 
	.groups = cxl_memdev_attribute_groups,

};



static void cxl_memdev_shutdown(struct cxl_memdev *cxlmd)

{

	down_write(&cxl_memdev_rwsem);

	cxlmd->cxlm = NULL;

	up_write(&cxl_memdev_rwsem);

}



static void cxl_memdev_unregister(void *_cxlmd)

{

	struct cxl_memdev *cxlmd = _cxlmd;

	struct device *dev = &cxlmd->dev;

	struct cdev *cdev = &cxlmd->cdev;

	const struct cdevm_file_operations *cdevm_fops;



	cdevm_fops = container_of(cdev->ops, typeof(*cdevm_fops), fops);

	cdevm_fops->shutdown(dev);



	cdev_device_del(&cxlmd->cdev, dev);

	cxl_memdev_shutdown(cxlmd);

	put_device(dev);

}



static struct cxl_memdev *cxl_memdev_alloc(struct cxl_mem *cxlm)

static struct cxl_memdev *cxl_memdev_alloc(struct cxl_mem *cxlm,

					   const struct file_operations *fops)

{

	struct pci_dev *pdev = cxlm->pdev;

	struct cxl_memdev *cxlmd;
@@ -1324,7 +1334,7 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_mem *cxlm) 
	device_set_pm_not_required(dev);



	cdev = &cxlmd->cdev;

	cdev_init(cdev, &cxl_memdev_fops);

	cdev_init(cdev, fops);

	return cxlmd;



err:
@@ -1332,15 +1342,16 @@ static struct cxl_memdev *cxl_memdev_alloc(struct cxl_mem *cxlm) 
	return ERR_PTR(rc);

}



static struct cxl_memdev *devm_cxl_add_memdev(struct device *host,

					      struct cxl_mem *cxlm)

static struct cxl_memdev *

devm_cxl_add_memdev(struct device *host, struct cxl_mem *cxlm,

		    const struct cdevm_file_operations *cdevm_fops)

{

	struct cxl_memdev *cxlmd;

	struct device *dev;

	struct cdev *cdev;

	int rc;



	cxlmd = cxl_memdev_alloc(cxlm);

	cxlmd = cxl_memdev_alloc(cxlm, &cdevm_fops->fops);

	if (IS_ERR(cxlmd))

		return cxlmd;
@@ -1370,7 +1381,7 @@ static struct cxl_memdev *devm_cxl_add_memdev(struct device *host, 
	 * The cdev was briefly live, shutdown any ioctl operations that

	 * saw that state.

	 */

	cxl_memdev_shutdown(cxlmd);

	cdevm_fops->shutdown(dev);

	put_device(dev);

	return ERR_PTR(rc);

}
@@ -1611,7 +1622,7 @@ static int cxl_mem_probe(struct pci_dev *pdev, const struct pci_device_id *id) 
	if (rc)

		return rc;



	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlm);

	cxlmd = devm_cxl_add_memdev(&pdev->dev, cxlm, &cxl_memdev_fops);

	if (IS_ERR(cxlmd))

		return PTR_ERR(cxlmd);