From ed30c69dc727699218cf49ff6f6e2f533ba1e4a2 Mon Sep 17 00:00:00 2001 From: Thomas Tai Date: Thu, 27 Apr 2017 10:51:48 -0700 Subject: [PATCH] sparc64: fix cdev_put() use-after-free when unbinding an LDom After turning on slub_debug=P kernel option, a kernel panic happens when unbinding an LDom. This suggests that there is memory corruption. The memory corruption is caused by vlds_fops_release() freeing a memory structure containing a cdev. The cdev is needed by fs/file_table.c after the file is released. The common approach to solve this issue is to add a kobject member in the structure and set it to be the parent of cdev. The kobject is then responsible to free the structure when the reference count is zero. The reference solution is based on the following patch. https://patchwork.kernel.org/patch/8985881/ Orabug: 25911389 Signed-off-by: Thomas Tai Reviewed-By: Liam Merwick Reviewed-by: Shannon Nelson Reviewed-by: Tom Saeger Signed-off-by: Allen Pais --- drivers/char/vlds.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/drivers/char/vlds.c b/drivers/char/vlds.c index 5c95f094b98a..08ffa7c81b83 100644 --- a/drivers/char/vlds.c +++ b/drivers/char/vlds.c @@ -81,6 +81,9 @@ struct vlds_dev { /* list of all services for this vlds device */ struct list_head service_info_list; + + /* set as cdev parent kobject */ + struct kobject kobj; }; /* for convenience, alias to the vlds_dev for the SP device */ @@ -2054,10 +2057,9 @@ static void vlds_free_vlds_dev(struct vlds_dev *vlds) list_del(&vlds->list); vlds_data.num_vlds_dev_list--; - /* free it */ - kfree(vlds->int_name); + /* free memory when kobject's reference count is 0 */ mutex_destroy(&vlds->vlds_mutex); - kfree(vlds); + kobject_put(&vlds->kobj); return; } @@ -2289,6 +2291,20 @@ static int vlds_get_next_avail_minor(void) return i; } +static void vlds_free_kobject(struct kobject *kobj) +{ + struct vlds_dev *vlds = + container_of(kobj, struct vlds_dev, kobj); + + dprintk("Deallocating (%s)\n", vlds->int_name); + kfree(vlds->int_name); + kfree(vlds); +} + +static struct kobj_type vlds_ktype = { + .release = vlds_free_kobject, +}; + static int vlds_alloc_vlds_dev(char *int_name, char *dev_name, struct device *vdev_dev, const u64 domain_handle, struct vlds_dev **vldsp) @@ -2342,9 +2358,13 @@ static int vlds_alloc_vlds_dev(char *int_name, char *dev_name, vlds->ref_cnt = 0; vlds->removed = false; + /* setup kobj to free vlds, it will be the parent of cdev */ + kobject_init(&vlds->kobj, &vlds_ktype); + /* create/add the associated cdev */ cdev_init(&vlds->cdev, &vlds_fops); vlds->cdev.owner = THIS_MODULE; + vlds->cdev.kobj.parent = &vlds->kobj; rv = cdev_add(&vlds->cdev, devt, 1); if (rv != 0) { dprintk("cdev_add() failed.\n"); @@ -2383,12 +2403,13 @@ error: if (devt) cdev_del(&vlds->cdev); - if (vlds->int_name) - kfree(vlds->int_name); - if (vlds != NULL) { + kfree(vlds->int_name); mutex_destroy(&vlds->vlds_mutex); - kfree(vlds); + if (vlds->kobj.state_initialized) + kobject_put(&vlds->kobj); + else + kfree(vlds); } dprintk("dev alloc failed (rv=%d)\n", rv); -- 2.50.1