On Thu, May 11, 2017 at 11:58 PM, Ingo Molnar [off-list ref] wrote:

* Linus Torvalds [off-list ref] wrote:

quoted

On Thu, May 11, 2017 at 4:17 PM, Thomas Garnier [off-list ref] wrote:

quoted

Ingo: Do you want the change as-is? Would you like it to be optional?
What do you think?

I'm not ingo, but I don't like that patch. It's in the wrong place -
that system call return code is too timing-critical to add address
limit checks.

Now what I think you *could* do is:

 - make "set_fs()" actually set a work flag in the current thread flags

 - do the test in the slow-path (syscall_return_slowpath).

Yes, yes, that ends up being architecture-specific, but it's fairly simple.

And it only slows down the system calls that actually use "set_fs()".
Sure, it will slow those down a fair amount, but they are hopefully a
small subset of all cases.

How does that sound to people?  Thats' where we currently do that

        if (IS_ENABLED(CONFIG_PROVE_LOCKING) &&
            WARN(irqs_disabled(), "syscall %ld left IRQs disabled",
regs->orig_ax))
                local_irq_enable();

check too, which is a fairly similar issue.

I really like that idea and I'd be perfectly fine with that solution, because it
puts the overhead where the problem comes from, and adds an extra incentive for
code to move away from set_fs() facilities. Win-win.

Great, I will adapt the patch for that.

Thanks,

        Ingo

-- 
Thomas