Messages in this thread Patch in this message | | | From | Schspa Shi <> | Subject | [PATCH] umh: fix UAF when the process is being killed | Date | Tue, 15 Nov 2022 22:02:33 +0800 |
| |
When the process is killed, wait_for_completion_state will return with -ERESTARTSYS, and the completion variable in the stack will be freed. If the user-mode thread is complete at the same time, there will be a UAF.
Please refer to the following scenarios. T1 T2 ------------------------------------------------------------------ call_usermodehelper_exec call_usermodehelper_exec_async << do something >> umh_complete(sub_info); comp = xchg(&sub_info->complete, NULL); /* we got the completion */ << context switch >>
<< Being killed >> retval = wait_for_completion_state(sub_info->complete, state); if (!retval) goto wait_done;
if (wait & UMH_KILLABLE) { /* umh_complete() will see NULL and free sub_info */ if (xchg(&sub_info->complete, NULL)) goto unlock; << we can't got the completion >> } .... unlock: helper_unlock(); return retval; }
/** * the completion variable in stack is end of life cycle. * and maybe freed due to process is recycled. */ --------UAF here---------- if (comp) complete(comp);
To fix it, we can put the completion variable in the subprocess_info variable.
Reported-by: syzbot+10d19d528d9755d9af22@syzkaller.appspotmail.com Reported-by: syzbot+70d5d5d83d03db2c813d@syzkaller.appspotmail.com Reported-by: syzbot+83cb0411d0fcf0a30fc1@syzkaller.appspotmail.com
Signed-off-by: Schspa Shi <schspa@gmail.com> --- include/linux/umh.h | 1 + kernel/umh.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/linux/umh.h b/include/linux/umh.h index 5d1f6129b847..801f7efbc825 100644 --- a/include/linux/umh.h +++ b/include/linux/umh.h @@ -20,6 +20,7 @@ struct file; struct subprocess_info { struct work_struct work; struct completion *complete; + struct completion done; const char *path; char **argv; char **envp; diff --git a/kernel/umh.c b/kernel/umh.c index 850631518665..3ed39956c777 100644 --- a/kernel/umh.c +++ b/kernel/umh.c @@ -380,6 +380,7 @@ struct subprocess_info *call_usermodehelper_setup(const char *path, char **argv, sub_info->cleanup = cleanup; sub_info->init = init; sub_info->data = data; + init_completion(&sub_info->done); out: return sub_info; } @@ -405,7 +406,6 @@ EXPORT_SYMBOL(call_usermodehelper_setup); int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) { unsigned int state = TASK_UNINTERRUPTIBLE; - DECLARE_COMPLETION_ONSTACK(done); int retval = 0; if (!sub_info->path) { @@ -431,7 +431,7 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) * This makes it possible to use umh_complete to free * the data structure in case of UMH_NO_WAIT. */ - sub_info->complete = (wait == UMH_NO_WAIT) ? NULL : &done; + sub_info->complete = (wait == UMH_NO_WAIT) ? NULL : &sub_info->done; sub_info->wait = wait; queue_work(system_unbound_wq, &sub_info->work); @@ -444,7 +444,7 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) if (wait & UMH_FREEZABLE) state |= TASK_FREEZABLE; - retval = wait_for_completion_state(&done, state); + retval = wait_for_completion_state(sub_info->complete, state); if (!retval) goto wait_done; -- 2.37.3
| |