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();
> +}
