On 07.11.2017 13:01, Stanislav Kinsburskiy wrote: > Sure, here it is: > > CPU #0 CPU#1 > > cleanup_mnt nfs41_callback_svc > ... (get transport from the list) > nfs_callback_down ... > ... ... > svc_close_net ... > ... ... > svc_xprt_free ... > svc_bc_sock_free bc_svc_process > kfree(xprt) svc_process_common > rqstp->rq_xprt->xpt_ops (use > after free) > > > > 07.11.2017 10:49, Kirill Tkhai пишет: >> On 03.11.2017 19:47, Stanislav Kinsburskiy wrote: >>> From: Stanislav Kinsburskiy <skinsbur...@parallels.com> >>> >>> The problem is that per-net SUNRPC transports shutdown is done regardless >>> current callback execution. This is a race leading to transport >>> use-after-free >>> in callback handler. >> >> Could you please draw the race to show the interaction between functions? >> >>> This patch fixes it in stright-forward way. I.e. it protects callback >>> execution with the same mutex used for per-net data creation and >>> destruction. >>> Hopefully, it won't slow down NFS client significantly. >>> >>> https://jira.sw.ru/browse/PSBM-75751 >>> >>> Signed-off-by: Stanislav Kinsburskiy <skinsbur...@parallels.com> >>> --- >>> fs/nfs/callback.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c >>> index 0beb275..82e8ed1 100644 >>> --- a/fs/nfs/callback.c >>> +++ b/fs/nfs/callback.c >>> @@ -118,6 +118,7 @@ nfs41_callback_svc(void *vrqstp) >>> continue; >>> >>> prepare_to_wait(&serv->sv_cb_waitq, &wq, TASK_INTERRUPTIBLE); >>> + mutex_lock(&nfs_callback_mutex); >>> spin_lock_bh(&serv->sv_cb_lock); >>> if (!list_empty(&serv->sv_cb_list)) { >>> req = list_first_entry(&serv->sv_cb_list, >>> @@ -129,8 +130,10 @@ nfs41_callback_svc(void *vrqstp) >>> error = bc_svc_process(serv, req, rqstp); >>> dprintk("bc_svc_process() returned w/ error code= %d\n", >>> error); >>> + mutex_unlock(&nfs_callback_mutex); >>> } else { >>> spin_unlock_bh(&serv->sv_cb_lock); >>> + mutex_unlock(&nfs_callback_mutex); >>> schedule(); >>> finish_wait(&serv->sv_cb_waitq, &wq); >>> } >> >> Couldn't this change introduce a deadlock like below? >> [thread] >> nfs_callback_down() >> nfs41_callback_svc() >> mutex_lock(&nfs_callback_mutex); >> kthread_stop(cb_info->task); >> wake_up_process(); >> wait_for_completion(&kthread->exited);
And what about above one? _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel
