| Message ID | 20170728145355.17258-1-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/28/2017 10:53 AM, Juergen Gross wrote: > When starting the xenwatch thread a theoretical deadlock situation is > possible: > > xs_init() contains: > > task = kthread_run(xenwatch_thread, NULL, "xenwatch"); > if (IS_ERR(task)) > return PTR_ERR(task); > xenwatch_pid = task->pid; > > And xenwatch_thread() does: > > mutex_lock(&xenwatch_mutex); > ... > event->handle->callback(); > ... > mutex_unlock(&xenwatch_mutex); > > The callback could call unregister_xenbus_watch() which does: > > ... > if (current->pid != xenwatch_pid) > mutex_lock(&xenwatch_mutex); > ... > > In case a watch is firing before xenwatch_pid could be set and the > callback of that watch unregisters a watch, then a self-deadlock would > occur. > > Avoid this by setting xenwatch_pid in xenwatch_thread(). > > Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> +stable?
On 28/07/17 17:14, Boris Ostrovsky wrote: > On 07/28/2017 10:53 AM, Juergen Gross wrote: >> When starting the xenwatch thread a theoretical deadlock situation is >> possible: >> >> xs_init() contains: >> >> task = kthread_run(xenwatch_thread, NULL, "xenwatch"); >> if (IS_ERR(task)) >> return PTR_ERR(task); >> xenwatch_pid = task->pid; >> >> And xenwatch_thread() does: >> >> mutex_lock(&xenwatch_mutex); >> ... >> event->handle->callback(); >> ... >> mutex_unlock(&xenwatch_mutex); >> >> The callback could call unregister_xenbus_watch() which does: >> >> ... >> if (current->pid != xenwatch_pid) >> mutex_lock(&xenwatch_mutex); >> ... >> >> In case a watch is firing before xenwatch_pid could be set and the >> callback of that watch unregisters a watch, then a self-deadlock would >> occur. >> >> Avoid this by setting xenwatch_pid in xenwatch_thread(). >> >> Signed-off-by: Juergen Gross <jgross@suse.com> > > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> > > +stable? As this problem is purely theoretical, I don't think the patch is appropriate for stable (at least the stable rules tell me so). Juergen
diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c index e46080214955..3e59590c7254 100644 --- a/drivers/xen/xenbus/xenbus_xs.c +++ b/drivers/xen/xenbus/xenbus_xs.c @@ -857,6 +857,8 @@ static int xenwatch_thread(void *unused) struct list_head *ent; struct xs_watch_event *event; + xenwatch_pid = current->pid; + for (;;) { wait_event_interruptible(watch_events_waitq, !list_empty(&watch_events)); @@ -925,7 +927,6 @@ int xs_init(void) task = kthread_run(xenwatch_thread, NULL, "xenwatch"); if (IS_ERR(task)) return PTR_ERR(task); - xenwatch_pid = task->pid; /* shutdown watches for kexec boot */ xs_reset_watches();
When starting the xenwatch thread a theoretical deadlock situation is possible: xs_init() contains: task = kthread_run(xenwatch_thread, NULL, "xenwatch"); if (IS_ERR(task)) return PTR_ERR(task); xenwatch_pid = task->pid; And xenwatch_thread() does: mutex_lock(&xenwatch_mutex); ... event->handle->callback(); ... mutex_unlock(&xenwatch_mutex); The callback could call unregister_xenbus_watch() which does: ... if (current->pid != xenwatch_pid) mutex_lock(&xenwatch_mutex); ... In case a watch is firing before xenwatch_pid could be set and the callback of that watch unregisters a watch, then a self-deadlock would occur. Avoid this by setting xenwatch_pid in xenwatch_thread(). Signed-off-by: Juergen Gross <jgross@suse.com> --- drivers/xen/xenbus/xenbus_xs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)