| Message ID | 51a4155c5bc2ca847a9cbe85c1c11918bb193141.1564086017.git.jpoimboe@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Remove redundant user_access_end() from __copy_from_user() error path | expand |
On Thu, 25 Jul 2019, Josh Poimboeuf wrote: > Objtool reports: > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x36: redundant UACCESS disable > > __copy_from_user() already does both STAC and CLAC, so the > user_access_end() in its error path adds an extra unnecessary CLAC. > > Fixes: 0b2c8f8b6b0c ("i915: fix missing user_access_end() in page fault exception case") > Reported-by: Thomas Gleixner <tglx@linutronix.de> > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > Link: https://github.com/ClangBuiltLinux/linux/issues/617 > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Quoting Thomas Gleixner (2019-07-25 22:55:45) > On Thu, 25 Jul 2019, Josh Poimboeuf wrote: > > > Objtool reports: > > > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x36: redundant UACCESS disable > > > > __copy_from_user() already does both STAC and CLAC, so the > > user_access_end() in its error path adds an extra unnecessary CLAC. > > > > Fixes: 0b2c8f8b6b0c ("i915: fix missing user_access_end() in page fault exception case") > > Reported-by: Thomas Gleixner <tglx@linutronix.de> > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > > Link: https://github.com/ClangBuiltLinux/linux/issues/617 > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Which tree do you plan to apply it to? I can put in drm-intel, and with the fixes tag it will percolate through to 5.3 and beyond, but if you want to apply it directly to squash the build warnings, feel free. -Chris
On Fri, 26 Jul 2019, Chris Wilson wrote: > Quoting Thomas Gleixner (2019-07-25 22:55:45) > > On Thu, 25 Jul 2019, Josh Poimboeuf wrote: > > > > > Objtool reports: > > > > > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x36: redundant UACCESS disable > > > > > > __copy_from_user() already does both STAC and CLAC, so the > > > user_access_end() in its error path adds an extra unnecessary CLAC. > > > > > > Fixes: 0b2c8f8b6b0c ("i915: fix missing user_access_end() in page fault exception case") > > > Reported-by: Thomas Gleixner <tglx@linutronix.de> > > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/617 > > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > > > Reviewed-by: Thomas Gleixner <tglx@linutronix.de> > > Which tree do you plan to apply it to? I can put in drm-intel, and with > the fixes tag it will percolate through to 5.3 and beyond, but if you > want to apply it directly to squash the build warnings, feel free. It would be nice to get it into 5.3. I can route it linuxwards if you give an Acked-by, but I'm happy to hand it to you :) Thanks, tglx
Quoting Thomas Gleixner (2019-07-26 20:18:32) > On Fri, 26 Jul 2019, Chris Wilson wrote: > > Quoting Thomas Gleixner (2019-07-25 22:55:45) > > > On Thu, 25 Jul 2019, Josh Poimboeuf wrote: > > > > > > > Objtool reports: > > > > > > > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x36: redundant UACCESS disable > > > > > > > > __copy_from_user() already does both STAC and CLAC, so the > > > > user_access_end() in its error path adds an extra unnecessary CLAC. > > > > > > > > Fixes: 0b2c8f8b6b0c ("i915: fix missing user_access_end() in page fault exception case") > > > > Reported-by: Thomas Gleixner <tglx@linutronix.de> > > > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > > > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > > > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > > > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/617 > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > > > > > Reviewed-by: Thomas Gleixner <tglx@linutronix.de> > > > > Which tree do you plan to apply it to? I can put in drm-intel, and with > > the fixes tag it will percolate through to 5.3 and beyond, but if you > > want to apply it directly to squash the build warnings, feel free. > > It would be nice to get it into 5.3. I can route it linuxwards if you give > an Acked-by, but I'm happy to hand it to you :) Acked-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Fri, Jul 26, 2019 at 9:30 PM Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Quoting Thomas Gleixner (2019-07-26 20:18:32) > > On Fri, 26 Jul 2019, Chris Wilson wrote: > > > Quoting Thomas Gleixner (2019-07-25 22:55:45) > > > > On Thu, 25 Jul 2019, Josh Poimboeuf wrote: > > > > > > > > > Objtool reports: > > > > > > > > > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x36: redundant UACCESS disable > > > > > > > > > > __copy_from_user() already does both STAC and CLAC, so the > > > > > user_access_end() in its error path adds an extra unnecessary CLAC. > > > > > > > > > > Fixes: 0b2c8f8b6b0c ("i915: fix missing user_access_end() in page fault exception case") > > > > > Reported-by: Thomas Gleixner <tglx@linutronix.de> > > > > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > > > > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > > > > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > > > > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/617 > > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > > > > > > > Reviewed-by: Thomas Gleixner <tglx@linutronix.de> > > > > > > Which tree do you plan to apply it to? I can put in drm-intel, and with > > > the fixes tag it will percolate through to 5.3 and beyond, but if you > > > want to apply it directly to squash the build warnings, feel free. > > > > It would be nice to get it into 5.3. I can route it linuxwards if you give > > an Acked-by, but I'm happy to hand it to you :) > > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> Thomas did you take this through tip tree after Chris' ACK? - Sedat -
On Wed, Jul 31, 2019 at 2:25 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > On Fri, Jul 26, 2019 at 9:30 PM Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > Quoting Thomas Gleixner (2019-07-26 20:18:32) > > > On Fri, 26 Jul 2019, Chris Wilson wrote: > > > > Quoting Thomas Gleixner (2019-07-25 22:55:45) > > > > > On Thu, 25 Jul 2019, Josh Poimboeuf wrote: > > > > > > > > > > > Objtool reports: > > > > > > > > > > > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x36: redundant UACCESS disable > > > > > > > > > > > > __copy_from_user() already does both STAC and CLAC, so the > > > > > > user_access_end() in its error path adds an extra unnecessary CLAC. > > > > > > > > > > > > Fixes: 0b2c8f8b6b0c ("i915: fix missing user_access_end() in page fault exception case") > > > > > > Reported-by: Thomas Gleixner <tglx@linutronix.de> > > > > > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > > > > > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > > > > > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > > > > > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/617 > > > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > > > > > > > > > Reviewed-by: Thomas Gleixner <tglx@linutronix.de> > > > > > > > > Which tree do you plan to apply it to? I can put in drm-intel, and with > > > > the fixes tag it will percolate through to 5.3 and beyond, but if you > > > > want to apply it directly to squash the build warnings, feel free. > > > > > > It would be nice to get it into 5.3. I can route it linuxwards if you give > > > an Acked-by, but I'm happy to hand it to you :) > > > > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > > Thomas did you take this through tip tree after Chris' ACK? > Hi, Gentle ping... Thomas and Chris: Will someone of you pick this up? As "objtool: Improve UACCESS coverage" [1] went trough tip tree I highly appreciate to do so with this one. Thanks. Regards, - Sedat - [1] https://git.kernel.org/linus/882a0db9d143e5e8dac54b96e83135bccd1f68d1 [2] https://github.com/ClangBuiltLinux/linux/issues/617
On Mon, Aug 05, 2019 at 09:29:53PM +0200, Sedat Dilek wrote: > On Wed, Jul 31, 2019 at 2:25 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > > > On Fri, Jul 26, 2019 at 9:30 PM Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > > > Quoting Thomas Gleixner (2019-07-26 20:18:32) > > > > On Fri, 26 Jul 2019, Chris Wilson wrote: > > > > > Quoting Thomas Gleixner (2019-07-25 22:55:45) > > > > > > On Thu, 25 Jul 2019, Josh Poimboeuf wrote: > > > > > > > > > > > > > Objtool reports: > > > > > > > > > > > > > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x36: redundant UACCESS disable > > > > > > > > > > > > > > __copy_from_user() already does both STAC and CLAC, so the > > > > > > > user_access_end() in its error path adds an extra unnecessary CLAC. > > > > > > > > > > > > > > Fixes: 0b2c8f8b6b0c ("i915: fix missing user_access_end() in page fault exception case") > > > > > > > Reported-by: Thomas Gleixner <tglx@linutronix.de> > > > > > > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > > > > > > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > > > > > > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > > > > > > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/617 > > > > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > > > > > > > > > > > Reviewed-by: Thomas Gleixner <tglx@linutronix.de> > > > > > > > > > > Which tree do you plan to apply it to? I can put in drm-intel, and with > > > > > the fixes tag it will percolate through to 5.3 and beyond, but if you > > > > > want to apply it directly to squash the build warnings, feel free. > > > > > > > > It would be nice to get it into 5.3. I can route it linuxwards if you give > > > > an Acked-by, but I'm happy to hand it to you :) > > > > > > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > Thomas did you take this through tip tree after Chris' ACK? > > > > Hi, > > Gentle ping... > Thomas and Chris: Will someone of you pick this up? > As "objtool: Improve UACCESS coverage" [1] went trough tip tree I > highly appreciate to do so with this one. I think Thomas has gone on holiday, so hopefully Chris can pick it up after all.
On Tue, Aug 6, 2019 at 5:59 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On Mon, Aug 05, 2019 at 09:29:53PM +0200, Sedat Dilek wrote: > > On Wed, Jul 31, 2019 at 2:25 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > > > > > On Fri, Jul 26, 2019 at 9:30 PM Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > > > > > Quoting Thomas Gleixner (2019-07-26 20:18:32) > > > > > On Fri, 26 Jul 2019, Chris Wilson wrote: > > > > > > Quoting Thomas Gleixner (2019-07-25 22:55:45) > > > > > > > On Thu, 25 Jul 2019, Josh Poimboeuf wrote: > > > > > > > > > > > > > > > Objtool reports: > > > > > > > > > > > > > > > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x36: redundant UACCESS disable > > > > > > > > > > > > > > > > __copy_from_user() already does both STAC and CLAC, so the > > > > > > > > user_access_end() in its error path adds an extra unnecessary CLAC. > > > > > > > > > > > > > > > > Fixes: 0b2c8f8b6b0c ("i915: fix missing user_access_end() in page fault exception case") > > > > > > > > Reported-by: Thomas Gleixner <tglx@linutronix.de> > > > > > > > > Reported-by: Sedat Dilek <sedat.dilek@gmail.com> > > > > > > > > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > > > > > > > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > > > > > > > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > > > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/617 > > > > > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > > > > > > > > > > > > > Reviewed-by: Thomas Gleixner <tglx@linutronix.de> > > > > > > > > > > > > Which tree do you plan to apply it to? I can put in drm-intel, and with > > > > > > the fixes tag it will percolate through to 5.3 and beyond, but if you > > > > > > want to apply it directly to squash the build warnings, feel free. > > > > > > > > > > It would be nice to get it into 5.3. I can route it linuxwards if you give > > > > > an Acked-by, but I'm happy to hand it to you :) > > > > > > > > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > > > Thomas did you take this through tip tree after Chris' ACK? > > > > > > > Hi, > > > > Gentle ping... > > Thomas and Chris: Will someone of you pick this up? > > As "objtool: Improve UACCESS coverage" [1] went trough tip tree I > > highly appreciate to do so with this one. > > I think Thomas has gone on holiday, so hopefully Chris can pick it up > after all. tglx just picked up 2 other patches of mine, bumping just in case he's not picking up patches while on vacation. ;)
On Thu, 8 Aug 2019, Nick Desaulniers wrote: > On Tue, Aug 6, 2019 at 5:59 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > Gentle ping... > > > Thomas and Chris: Will someone of you pick this up? > > > As "objtool: Improve UACCESS coverage" [1] went trough tip tree I > > > highly appreciate to do so with this one. > > > > I think Thomas has gone on holiday, so hopefully Chris can pick it up > > after all. > > tglx just picked up 2 other patches of mine, bumping just in case he's > not picking up patches while on vacation. ;) I'm only half on vacation :) So I can pick it up.
On Thu, Aug 8, 2019 at 1:22 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > tglx just picked up 2 other patches of mine, bumping just in case he's > > not picking up patches while on vacation. ;) > > I'm only half on vacation :) > > So I can pick it up. Thanks, will send half margaritas.
On Fri, Aug 9, 2019 at 1:03 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Thu, Aug 8, 2019 at 1:22 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > > tglx just picked up 2 other patches of mine, bumping just in case he's > > > not picking up patches while on vacation. ;) > > > > I'm only half on vacation :) > > > > So I can pick it up. > > Thanks, will send half margaritas. > Sends some Turkish Cay. - Sedat -
On Fri, 9 Aug 2019, Sedat Dilek wrote: > On Fri, Aug 9, 2019 at 1:03 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > On Thu, Aug 8, 2019 at 1:22 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > > > tglx just picked up 2 other patches of mine, bumping just in case he's > > > > not picking up patches while on vacation. ;) > > > > > > I'm only half on vacation :) > > > > > > So I can pick it up. > > > > Thanks, will send half margaritas. > > > > Sends some Turkish Cay. One day, I'm going to collect all the things people promised to send or buy me in the past 15 years. That's going to be a really huge party :)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 5fae0e50aad0..41dab9ea33cd 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -1628,6 +1628,7 @@ static int check_relocations(const struct drm_i915_gem_exec_object2 *entry) static int eb_copy_relocations(const struct i915_execbuffer *eb) { + struct drm_i915_gem_relocation_entry *relocs; const unsigned int count = eb->buffer_count; unsigned int i; int err; @@ -1635,7 +1636,6 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb) for (i = 0; i < count; i++) { const unsigned int nreloc = eb->exec[i].relocation_count; struct drm_i915_gem_relocation_entry __user *urelocs; - struct drm_i915_gem_relocation_entry *relocs; unsigned long size; unsigned long copied; @@ -1663,14 +1663,8 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb) if (__copy_from_user((char *)relocs + copied, (char __user *)urelocs + copied, - len)) { -end_user: - user_access_end(); -end: - kvfree(relocs); - err = -EFAULT; - goto err; - } + len)) + goto end; copied += len; } while (copied < size); @@ -1699,10 +1693,14 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb) return 0; +end_user: + user_access_end(); +end: + kvfree(relocs); + err = -EFAULT; err: while (i--) { - struct drm_i915_gem_relocation_entry *relocs = - u64_to_ptr(typeof(*relocs), eb->exec[i].relocs_ptr); + relocs = u64_to_ptr(typeof(*relocs), eb->exec[i].relocs_ptr); if (eb->exec[i].relocation_count) kvfree(relocs); }
Objtool reports: drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: .altinstr_replacement+0x36: redundant UACCESS disable __copy_from_user() already does both STAC and CLAC, so the user_access_end() in its error path adds an extra unnecessary CLAC. Fixes: 0b2c8f8b6b0c ("i915: fix missing user_access_end() in page fault exception case") Reported-by: Thomas Gleixner <tglx@linutronix.de> Reported-by: Sedat Dilek <sedat.dilek@gmail.com> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> Tested-by: Nick Desaulniers <ndesaulniers@google.com> Tested-by: Sedat Dilek <sedat.dilek@gmail.com> Link: https://github.com/ClangBuiltLinux/linux/issues/617 Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-)