Skip to content

Commit

Permalink
cxl/pci: Introduce cdevm_file_operations
Browse files Browse the repository at this point in the history
[ Upstream commit 9cc238c ]

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: Ben Widawsky <ben.widawsky@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Link: https://lore.kernel.org/r/162792539962.368511.2962268954245340288.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
djbw authored and gregkh committed Sep 26, 2021
1 parent e921d59 commit 964a983
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 27 deletions.
15 changes: 15 additions & 0 deletions drivers/cxl/cxlmem.h
Expand Up @@ -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
Expand Down
65 changes: 38 additions & 27 deletions drivers/cxl/pci.c
Expand Up @@ -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 = {
.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,
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)
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -1324,23 +1334,24 @@ 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:
kfree(cxlmd);
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;

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit 964a983

Please sign in to comment.