Re: [RESEND PATCH] IB/umad: Fix a use-after-free

2014-06-06 Thread Roland Dreier
On Fri, Jun 6, 2014 at 9:25 AM, Yann Droneaud  wrote:
> It seems this one was not rolled in your latest katamari[1].


Thanks, picked it up this time around.

 - R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RESEND PATCH] IB/umad: Fix a use-after-free

2014-06-06 Thread Yann Droneaud
From: Bart Van Assche 

Avoid that closing /dev/infiniband/umad or /dev/infiniband/issm
triggers a use-after-free. __fput() in fs/file_table.c invokes
f_op->release() before it invokes cdev_put(). Make sure that the
ib_umad_device structure is freed by the cdev_put() call instead of
f_op->release(). This avoids that changing the port mode from IB into
Ethernet and back to IB followed by restarting opensmd triggers the
following kernel oops:

general protection fault:  [#1] PREEMPT SMP
RIP: 0010:[]  [] module_put+0x2c/0x170
Call Trace:
 [] cdev_put+0x20/0x30
 [] __fput+0x1ae/0x1f0
 [] fput+0xe/0x10
 [] task_work_run+0xac/0xe0
 [] do_notify_resume+0x9f/0xc0
 [] int_signal+0x12/0x17

Reference: https://bugzilla.kernel.org/show_bug.cgi?id=75051
Signed-off-by: Bart Van Assche 
Reviewed-by: Yann Droneaud 
Cc: Alex Chiang 
Cc:  # 3.x: 8ec0a0e6b58: IB/umad: Fix error handling
---
Hi Bart and Roland,

It seems this one was not rolled in your latest katamari[1].
Since the original patch didn't apply on top of commit
8ec0a0e6b58 ('IB/umad: Fix error handling'), I've applied it
manually and resend it for review and inclusion.

I hope having correctly set the Cc: sta...@vger.kernel.org
tag to state the dependency on earlier patch.

[1] this term is actually used by Xorg to team to describe
the process of gathering each little tiny piece needed
for a release :) By the way, it's also a video game.

Regards.

 drivers/infiniband/core/user_mad.c | 30 +++---
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/core/user_mad.c 
b/drivers/infiniband/core/user_mad.c
index 9bdf576cd4ab..1acb99100556 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -98,7 +98,7 @@ struct ib_umad_port {
 
 struct ib_umad_device {
int  start_port, end_port;
-   struct kref  ref;
+   struct kobject   kobj;
struct ib_umad_port  port[0];
 };
 
@@ -134,14 +134,18 @@ static DECLARE_BITMAP(dev_map, IB_UMAD_MAX_PORTS);
 static void ib_umad_add_one(struct ib_device *device);
 static void ib_umad_remove_one(struct ib_device *device);
 
-static void ib_umad_release_dev(struct kref *ref)
+static void ib_umad_release_dev(struct kobject *kobj)
 {
struct ib_umad_device *dev =
-   container_of(ref, struct ib_umad_device, ref);
+   container_of(kobj, struct ib_umad_device, kobj);
 
kfree(dev);
 }
 
+static struct kobj_type ib_umad_dev_ktype = {
+   .release = ib_umad_release_dev,
+};
+
 static int hdr_size(struct ib_umad_file *file)
 {
return file->use_pkey_index ? sizeof (struct ib_user_mad_hdr) :
@@ -812,7 +816,7 @@ static int ib_umad_open(struct inode *inode, struct file 
*filp)
goto out;
}
 
-   kref_get(&port->umad_dev->ref);
+   kobject_get(&port->umad_dev->kobj);
 
 out:
mutex_unlock(&port->file_mutex);
@@ -851,7 +855,7 @@ static int ib_umad_close(struct inode *inode, struct file 
*filp)
mutex_unlock(&file->port->file_mutex);
 
kfree(file);
-   kref_put(&dev->ref, ib_umad_release_dev);
+   kobject_put(&dev->kobj);
 
return 0;
 }
@@ -902,7 +906,7 @@ static int ib_umad_sm_open(struct inode *inode, struct file 
*filp)
if (ret)
goto err_clr_sm_cap;
 
-   kref_get(&port->umad_dev->ref);
+   kobject_get(&port->umad_dev->kobj);
 
return 0;
 
@@ -932,7 +936,7 @@ static int ib_umad_sm_close(struct inode *inode, struct 
file *filp)
 
up(&port->sm_sem);
 
-   kref_put(&port->umad_dev->ref, ib_umad_release_dev);
+   kobject_put(&port->umad_dev->kobj);
 
return ret;
 }
@@ -1000,6 +1004,7 @@ static int find_overflow_devnum(void)
 }
 
 static int ib_umad_init_port(struct ib_device *device, int port_num,
+struct ib_umad_device *umad_dev,
 struct ib_umad_port *port)
 {
int devnum;
@@ -1032,6 +1037,7 @@ static int ib_umad_init_port(struct ib_device *device, 
int port_num,
 
cdev_init(&port->cdev, &umad_fops);
port->cdev.owner = THIS_MODULE;
+   port->cdev.kobj.parent = &umad_dev->kobj;
kobject_set_name(&port->cdev.kobj, "umad%d", port->dev_num);
if (cdev_add(&port->cdev, base, 1))
goto err_cdev;
@@ -1050,6 +1056,7 @@ static int ib_umad_init_port(struct ib_device *device, 
int port_num,
base += IB_UMAD_MAX_PORTS;
cdev_init(&port->sm_cdev, &umad_sm_fops);
port->sm_cdev.owner = THIS_MODULE;
+   port->sm_cdev.kobj.parent = &umad_dev->kobj;
kobject_set_name(&port->sm_cdev.kobj, "issm%d", port->dev_num);
if (cdev_add(&port->sm_cdev, base, 1))
goto err_sm_cdev;
@@ -1143,7 +1150,7 @@ static void ib_umad_add_one(struct ib_device *device)
if (!umad_dev)
return;
 
-   kref_init(&umad_dev->ref);
+   kobject_init(&umad_dev->kobj, &ib_um