2017-07-28 14:54:02

by Jürgen Groß

[permalink] [raw]
Subject: [PATCH] xen: avoid deadlock in xenbus

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 <[email protected]>
---
drivers/xen/xenbus/xenbus_xs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

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();
--
2.12.3


2017-07-28 15:13:31

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH] xen: avoid deadlock in xenbus

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 <[email protected]>

Reviewed-by: Boris Ostrovsky <[email protected]>

+stable?

2017-07-28 15:16:37

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH] xen: avoid deadlock in xenbus

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 <[email protected]>
>
> Reviewed-by: Boris Ostrovsky <[email protected]>
>
> +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