Re: [RFC patch 09/15] entry: Provide generic exit to usermode functionality

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Sep 19, 2019 at 05:03:23PM +0200, Thomas Gleixner wrote:
> +static unsigned long core_exit_to_usermode_work(struct pt_regs *regs,
> +						unsigned long ti_work)
> +{
> +	/*
> +	 * Before returning to user space ensure that all pending work
> +	 * items have been completed.
> +	 */
> +	while (ti_work & EXIT_TO_USERMODE_WORK) {
> +
> +		local_irq_enable_exit_to_user(ti_work);
> +
> +		if (ti_work & _TIF_NEED_RESCHED)
> +			schedule();
> +
> +		if (ti_work & _TIF_UPROBE)
> +			uprobe_notify_resume(regs);
> +
> +		if (ti_work & _TIF_PATCH_PENDING)
> +			klp_update_patch_state(current);
> +
> +		if (ti_work & _TIF_SIGPENDING)
> +			arch_do_signal(regs);
> +
> +		if (ti_work & _TIF_NOTIFY_RESUME) {
> +			clear_thread_flag(TIF_NOTIFY_RESUME);
> +			tracehook_notify_resume(regs);
> +			rseq_handle_notify_resume(NULL, regs);
> +		}
> +
> +		/* Architecture specific TIF work */
> +		arch_exit_to_usermode_work(regs, ti_work);
> +
> +		/*
> +		 * Disable interrupts and reevaluate the work flags as they
> +		 * might have changed while interrupts and preemption was
> +		 * enabled above.
> +		 */
> +		local_irq_disable_exit_to_user();
> +		ti_work = READ_ONCE(current_thread_info()->flags);
> +	}
> +	/*
> +	 * Was checked in exit_to_usermode_work() already, but the above
> +	 * loop might have wreckaged it.
> +	 */
> +	addr_limit_user_check();
> +	return ti_work;
> +}
> +
> +static void do_exit_to_usermode(struct pt_regs *regs)
> +{
> +	unsigned long ti_work = READ_ONCE(current_thread_info()->flags);
> +
> +	lockdep_sys_exit();
> +
> +	addr_limit_user_check();
> +
> +	if (unlikely(ti_work & EXIT_TO_USERMODE_WORK))
> +		ti_work = core_exit_to_usermode_work(regs, ti_work);

would it make sense to do:

	lockdep_sys_exit();
	addr_limit_user_check();

here instead of before core_exit_to_usermode_work(); that would also
allow getting rid of that second addr_limit_user_check() invocation.

And movind that lockdep check later would catch any of the
EXIT_TO_USERMODE_WORK users leaking a lock.

> +
> +	arch_exit_to_usermode(regs, ti_work);
> +	/* Return to userspace right after this which turns on interrupts */
> +	trace_hardirqs_on();
> +}