Commit 9420098a authored by Vitaly Kuznetsov's avatar Vitaly Kuznetsov Committed by Greg Kroah-Hartman

Drivers: hv: utils: fix crash when device is removed from host side

The crash is observed when a service is being disabled host side while
userspace daemon is connected to the device:

[   90.244859] general protection fault: 0000 [#1] SMP
...
[   90.800082] Call Trace:
[   90.800082]  [<ffffffff81187008>] __fput+0xc8/0x1f0
[   90.800082]  [<ffffffff8118716e>] ____fput+0xe/0x10
...
[   90.800082]  [<ffffffff81015278>] do_signal+0x28/0x580
[   90.800082]  [<ffffffff81086656>] ? finish_task_switch+0xa6/0x180
[   90.800082]  [<ffffffff81443ebf>] ? __schedule+0x28f/0x870
[   90.800082]  [<ffffffffa01ebbaa>] ? hvt_op_read+0x12a/0x140 [hv_utils]
...

The problem is that hvutil_transport_destroy() which does misc_deregister()
freeing the appropriate device is reachable by two paths: module unload
and from util_remove(). While module unload path is protected by .owner in
struct file_operations util_remove() path is not. Freeing the device while
someone holds an open fd for it is a show stopper.

In general, it is not possible to revoke an fd from all users so the only
way to solve the issue is to defer freeing the hvutil_transport structure.
Signed-off-by: default avatarVitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: default avatarK. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent a1502566
...@@ -155,13 +155,22 @@ static int hvt_op_open(struct inode *inode, struct file *file) ...@@ -155,13 +155,22 @@ static int hvt_op_open(struct inode *inode, struct file *file)
return ret; return ret;
} }
static void hvt_transport_free(struct hvutil_transport *hvt)
{
misc_deregister(&hvt->mdev);
kfree(hvt->outmsg);
kfree(hvt);
}
static int hvt_op_release(struct inode *inode, struct file *file) static int hvt_op_release(struct inode *inode, struct file *file)
{ {
struct hvutil_transport *hvt; struct hvutil_transport *hvt;
int mode_old;
hvt = container_of(file->f_op, struct hvutil_transport, fops); hvt = container_of(file->f_op, struct hvutil_transport, fops);
mutex_lock(&hvt->lock); mutex_lock(&hvt->lock);
mode_old = hvt->mode;
if (hvt->mode != HVUTIL_TRANSPORT_DESTROY) if (hvt->mode != HVUTIL_TRANSPORT_DESTROY)
hvt->mode = HVUTIL_TRANSPORT_INIT; hvt->mode = HVUTIL_TRANSPORT_INIT;
/* /*
...@@ -171,6 +180,9 @@ static int hvt_op_release(struct inode *inode, struct file *file) ...@@ -171,6 +180,9 @@ static int hvt_op_release(struct inode *inode, struct file *file)
hvt_reset(hvt); hvt_reset(hvt);
mutex_unlock(&hvt->lock); mutex_unlock(&hvt->lock);
if (mode_old == HVUTIL_TRANSPORT_DESTROY)
hvt_transport_free(hvt);
return 0; return 0;
} }
...@@ -304,17 +316,25 @@ struct hvutil_transport *hvutil_transport_init(const char *name, ...@@ -304,17 +316,25 @@ struct hvutil_transport *hvutil_transport_init(const char *name,
void hvutil_transport_destroy(struct hvutil_transport *hvt) void hvutil_transport_destroy(struct hvutil_transport *hvt)
{ {
int mode_old;
mutex_lock(&hvt->lock); mutex_lock(&hvt->lock);
mode_old = hvt->mode;
hvt->mode = HVUTIL_TRANSPORT_DESTROY; hvt->mode = HVUTIL_TRANSPORT_DESTROY;
wake_up_interruptible(&hvt->outmsg_q); wake_up_interruptible(&hvt->outmsg_q);
mutex_unlock(&hvt->lock); mutex_unlock(&hvt->lock);
/*
* In case we were in 'chardev' mode we still have an open fd so we
* have to defer freeing the device. Netlink interface can be freed
* now.
*/
spin_lock(&hvt_list_lock); spin_lock(&hvt_list_lock);
list_del(&hvt->list); list_del(&hvt->list);
spin_unlock(&hvt_list_lock); spin_unlock(&hvt_list_lock);
if (hvt->cn_id.idx > 0 && hvt->cn_id.val > 0) if (hvt->cn_id.idx > 0 && hvt->cn_id.val > 0)
cn_del_callback(&hvt->cn_id); cn_del_callback(&hvt->cn_id);
misc_deregister(&hvt->mdev);
kfree(hvt->outmsg); if (mode_old != HVUTIL_TRANSPORT_CHARDEV)
kfree(hvt); hvt_transport_free(hvt);
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment