Skip to content

Commit 1c3333a

Browse files
committed
cxl/mem: Do not rely on device_add() side effects for dev_set_name() failures
While device_add() will happen to catch dev_set_name() failures it is a broken pattern to follow given that the core may try to fall back to a different name. Add explicit checking for dev_set_name() failures to be cleaned up by put_device(). Skip cdev_device_add() and proceed directly to put_device() if the name set fails. This type of bug is easier to see if 'alloc' is split from 'add' operations that require put_device() on failure. So cxl_memdev_alloc() is split out as a result. Fixes: b39cb10 ("cxl/mem: Register CXL memX devices") Reported-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Link: https://lore.kernel.org/r/161728760514.2474381.1163928273337158134.stgit@dwillia2-desk3.amr.corp.intel.com Signed-off-by: Dan Williams <dan.j.williams@intel.com>
1 parent 5877515 commit 1c3333a

File tree

1 file changed

+29
-10
lines changed

1 file changed

+29
-10
lines changed

drivers/cxl/mem.c

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,7 +1180,7 @@ static void cxl_memdev_unregister(void *_cxlmd)
11801180
put_device(dev);
11811181
}
11821182

1183-
static int cxl_mem_add_memdev(struct cxl_mem *cxlm)
1183+
static struct cxl_memdev *cxl_memdev_alloc(struct cxl_mem *cxlm)
11841184
{
11851185
struct pci_dev *pdev = cxlm->pdev;
11861186
struct cxl_memdev *cxlmd;
@@ -1190,11 +1190,11 @@ static int cxl_mem_add_memdev(struct cxl_mem *cxlm)
11901190

11911191
cxlmd = kzalloc(sizeof(*cxlmd), GFP_KERNEL);
11921192
if (!cxlmd)
1193-
return -ENOMEM;
1193+
return ERR_PTR(-ENOMEM);
11941194

11951195
rc = ida_alloc_range(&cxl_memdev_ida, 0, CXL_MEM_MAX_DEVS, GFP_KERNEL);
11961196
if (rc < 0)
1197-
goto err_id;
1197+
goto err;
11981198
cxlmd->id = rc;
11991199

12001200
dev = &cxlmd->dev;
@@ -1203,34 +1203,53 @@ static int cxl_mem_add_memdev(struct cxl_mem *cxlm)
12031203
dev->bus = &cxl_bus_type;
12041204
dev->devt = MKDEV(cxl_mem_major, cxlmd->id);
12051205
dev->type = &cxl_memdev_type;
1206-
dev_set_name(dev, "mem%d", cxlmd->id);
12071206

12081207
cdev = &cxlmd->cdev;
12091208
cdev_init(cdev, &cxl_memdev_fops);
1209+
return cxlmd;
1210+
1211+
err:
1212+
kfree(cxlmd);
1213+
return ERR_PTR(rc);
1214+
}
1215+
1216+
static int cxl_mem_add_memdev(struct cxl_mem *cxlm)
1217+
{
1218+
struct cxl_memdev *cxlmd;
1219+
struct device *dev;
1220+
struct cdev *cdev;
1221+
int rc;
1222+
1223+
cxlmd = cxl_memdev_alloc(cxlm);
1224+
if (IS_ERR(cxlmd))
1225+
return PTR_ERR(cxlmd);
1226+
1227+
dev = &cxlmd->dev;
1228+
rc = dev_set_name(dev, "mem%d", cxlmd->id);
1229+
if (rc)
1230+
goto err;
12101231

12111232
/*
12121233
* Activate ioctl operations, no cxl_memdev_rwsem manipulation
12131234
* needed as this is ordered with cdev_add() publishing the device.
12141235
*/
12151236
cxlmd->cxlm = cxlm;
12161237

1238+
cdev = &cxlmd->cdev;
12171239
rc = cdev_device_add(cdev, dev);
12181240
if (rc)
1219-
goto err_add;
1241+
goto err;
12201242

12211243
return devm_add_action_or_reset(dev->parent, cxl_memdev_unregister,
12221244
cxlmd);
12231245

1224-
err_add:
1246+
err:
12251247
/*
12261248
* The cdev was briefly live, shutdown any ioctl operations that
12271249
* saw that state.
12281250
*/
12291251
cxl_memdev_shutdown(cxlmd);
1230-
ida_free(&cxl_memdev_ida, cxlmd->id);
1231-
err_id:
1232-
kfree(cxlmd);
1233-
1252+
put_device(dev);
12341253
return rc;
12351254
}
12361255

0 commit comments

Comments
 (0)