[CRIU] [PATCH 3/3] seccomp: Use BUG after CR_STATE_RESTORE_SIGCHLD stage if failed

[Cyrill Gorcunov]

On Thu, May 24, 2018 at 02:45:43PM -0700, Andrey Vagin wrote:
> > @@ -588,18 +589,23 @@ long __export_restore_thread(struct thread_restore_args *args)
> >  	 * operation bound to uid 0 in current user ns.
> >  	 */
> >  	if (restore_seccomp(args))
> > -		goto core_restore_end;
> > +		goto thread_panic;
> >  
> >  	ret = restore_creds(args->creds_args, args->ta->proc_fd);
> >  	ret = ret || restore_dumpable_flag(&args->ta->mm);
> >  	ret = ret || restore_pdeath_sig(args);
> > -	if (ret)
> > -		goto core_restore_end;
> >  
> >  	restore_finish_stage(task_entries_local, CR_STATE_RESTORE_CREDS);
> >  
> >  	futex_dec_and_wake(&thread_inprogress);
> >  
> > +	/*
> > +	 * We can't just exit in such deep stage on thread restore,
> > +	 * instead notify group leader that we've failed.
> > +	 */
> > +	if (ret)
> > +		goto thread_panic;
> 
> Why do you move this check after switching a stage?

Because the whoole idea was to not kill eveyone from inside
of thread but rather wait until group leader get final notification
that all threads are done and then kill the leader.

> > +	/*
> > +	 * If error happened somewhere insidee deep restore of
> > +	 * a thread where we can't exit gracefully and have fired
> > +	 * BUG already, do the same here in group leader so all
> > +	 * threads get killed as well.
> > +	 *
> > +	 * Note that we've to complete @thread_inprogress counting
> > +	 * since the group leader is still waiting and futex-abort
> > +	 * flag won't work here because the form we have futexes
> > +	 * doesn't allow multiple writers
> > +	 */
> > +	if (atomic_read(&thread_aborted))
> 
> I don't think that we need this check and BUG. I think when one thread
> calls BUG(), the whole process will be killed.

Yeah, overlooked. Anyway, drop it, we already have version with bug merged.