Re: [PATCH 6/8] i915,uaccess: Fix redundant CLAC

[Linus Torvalds]

On Thu, Feb 28, 2019 at 7:05 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> drivers/gpu/drm/i915/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x3c: redundant UACCESS disable
> drivers/gpu/drm/i915/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x66: redundant UACCESS disable
>
> AKA. you don't need user_access_end() if user_access_begin() fails.

NOOO!

This is complete garbage, and will end up running with AC set forever after.

PeterZ, you need to remove that "redundant UACCESS disable" check, or
make it a whole lot smarter. Because as it is, it's horribly horribly
wrong.

We absolutely _have_ to have that "user_access_end()" there.

Yes, it is redundant (but harmlessly so) if no fault occurs.

But if a fault occurs, that "user_access_end()" is what clears AC on
the faulting path. That's absolutely required, because we don't clear
it on return from exception (and we shouldn't - one common pattern for
user space exceptions is "try to do a big access, if that fails go
back to using small accesses until it fails again").

Your reachability clearly doesn't take exception handling into account.

That patch must die, and your incorrect reachability model must be fixed.

Right now the objtool rules seem to be worse than not using objtool,
if it causes these kinds of false positives.

              Linus