Messages in this thread | | | From | Linus Torvalds <> | Date | Thu, 28 Feb 2019 08:49:57 -0800 | Subject | Re: [PATCH 6/8] i915,uaccess: Fix redundant CLAC |
| |
On Thu, Feb 28, 2019 at 7:05 AM Peter Zijlstra <peterz@infradead.org> 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
| |