linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
@ 2019-07-25 20:29 Josh Poimboeuf
  2019-07-25 21:55 ` Thomas Gleixner
  2019-08-09 21:16 ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
  0 siblings, 2 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2019-07-25 20:29 UTC (permalink / raw)
  To: intel-gfx, dri-devel
  Cc: linux-kernel, Chris Wilson, Peter Zijlstra, Thomas Gleixner,
	Linus Torvalds, Sedat Dilek, Nick Desaulniers, Nathan Chancellor,
	Arnd Bergmann

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(-)

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);
 	}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-07-25 20:29 [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path Josh Poimboeuf
@ 2019-07-25 21:55 ` Thomas Gleixner
  2019-07-26 19:05   ` Chris Wilson
  2019-08-09 21:16 ` [tip:core/urgent] " tip-bot for Josh Poimboeuf
  1 sibling, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2019-07-25 21:55 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: intel-gfx, dri-devel, linux-kernel, Chris Wilson, Peter Zijlstra,
	Linus Torvalds, Sedat Dilek, Nick Desaulniers, Nathan Chancellor,
	Arnd Bergmann

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>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-07-25 21:55 ` Thomas Gleixner
@ 2019-07-26 19:05   ` Chris Wilson
  2019-07-26 19:18     ` Thomas Gleixner
  0 siblings, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2019-07-26 19:05 UTC (permalink / raw)
  To: Josh Poimboeuf, Thomas Gleixner
  Cc: intel-gfx, dri-devel, linux-kernel, Peter Zijlstra,
	Linus Torvalds, Sedat Dilek, Nick Desaulniers, Nathan Chancellor,
	Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-07-26 19:05   ` Chris Wilson
@ 2019-07-26 19:18     ` Thomas Gleixner
  2019-07-26 19:30       ` Chris Wilson
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2019-07-26 19:18 UTC (permalink / raw)
  To: Chris Wilson
  Cc: Josh Poimboeuf, intel-gfx, dri-devel, linux-kernel,
	Peter Zijlstra, Linus Torvalds, Sedat Dilek, Nick Desaulniers,
	Nathan Chancellor, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-07-26 19:18     ` Thomas Gleixner
@ 2019-07-26 19:30       ` Chris Wilson
  2019-07-31 12:25         ` Sedat Dilek
  0 siblings, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2019-07-26 19:30 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Josh Poimboeuf, intel-gfx, dri-devel, linux-kernel,
	Peter Zijlstra, Linus Torvalds, Sedat Dilek, Nick Desaulniers,
	Nathan Chancellor, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-07-26 19:30       ` Chris Wilson
@ 2019-07-31 12:25         ` Sedat Dilek
  2019-08-05 19:29           ` Sedat Dilek
  0 siblings, 1 reply; 14+ messages in thread
From: Sedat Dilek @ 2019-07-31 12:25 UTC (permalink / raw)
  To: Chris Wilson
  Cc: Thomas Gleixner, Josh Poimboeuf, intel-gfx, dri-devel,
	linux-kernel, Peter Zijlstra, Linus Torvalds, Nick Desaulniers,
	Nathan Chancellor, Arnd Bergmann

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 -

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-07-31 12:25         ` Sedat Dilek
@ 2019-08-05 19:29           ` Sedat Dilek
  2019-08-06 12:59             ` Josh Poimboeuf
  0 siblings, 1 reply; 14+ messages in thread
From: Sedat Dilek @ 2019-08-05 19:29 UTC (permalink / raw)
  To: Chris Wilson
  Cc: Thomas Gleixner, Josh Poimboeuf, intel-gfx, dri-devel,
	linux-kernel, Peter Zijlstra, Linus Torvalds, Nick Desaulniers,
	Nathan Chancellor, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-08-05 19:29           ` Sedat Dilek
@ 2019-08-06 12:59             ` Josh Poimboeuf
  2019-08-08 20:14               ` Nick Desaulniers
  0 siblings, 1 reply; 14+ messages in thread
From: Josh Poimboeuf @ 2019-08-06 12:59 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Chris Wilson, Thomas Gleixner, intel-gfx, dri-devel,
	linux-kernel, Peter Zijlstra, Linus Torvalds, Nick Desaulniers,
	Nathan Chancellor, Arnd Bergmann

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.

-- 
Josh

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-08-06 12:59             ` Josh Poimboeuf
@ 2019-08-08 20:14               ` Nick Desaulniers
  2019-08-08 20:21                 ` Thomas Gleixner
  0 siblings, 1 reply; 14+ messages in thread
From: Nick Desaulniers @ 2019-08-08 20:14 UTC (permalink / raw)
  To: Josh Poimboeuf, Thomas Gleixner
  Cc: Sedat Dilek, Chris Wilson, intel-gfx, dri-devel, LKML,
	Peter Zijlstra, Linus Torvalds, Nathan Chancellor, Arnd Bergmann

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. ;)
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-08-08 20:14               ` Nick Desaulniers
@ 2019-08-08 20:21                 ` Thomas Gleixner
  2019-08-08 23:02                   ` Nick Desaulniers
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Gleixner @ 2019-08-08 20:21 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Josh Poimboeuf, Sedat Dilek, Chris Wilson, intel-gfx, dri-devel,
	LKML, Peter Zijlstra, Linus Torvalds, Nathan Chancellor,
	Arnd Bergmann

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.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-08-08 20:21                 ` Thomas Gleixner
@ 2019-08-08 23:02                   ` Nick Desaulniers
  2019-08-09 20:19                     ` Sedat Dilek
  0 siblings, 1 reply; 14+ messages in thread
From: Nick Desaulniers @ 2019-08-08 23:02 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Josh Poimboeuf, Sedat Dilek, Chris Wilson, intel-gfx, dri-devel,
	LKML, Peter Zijlstra, Linus Torvalds, Nathan Chancellor,
	Arnd Bergmann

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.

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-08-08 23:02                   ` Nick Desaulniers
@ 2019-08-09 20:19                     ` Sedat Dilek
  2019-08-10  5:59                       ` Thomas Gleixner
  0 siblings, 1 reply; 14+ messages in thread
From: Sedat Dilek @ 2019-08-09 20:19 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Thomas Gleixner, Josh Poimboeuf, Chris Wilson, intel-gfx,
	dri-devel, LKML, Peter Zijlstra, Linus Torvalds,
	Nathan Chancellor, Arnd Bergmann

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 -

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tip:core/urgent] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-07-25 20:29 [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path Josh Poimboeuf
  2019-07-25 21:55 ` Thomas Gleixner
@ 2019-08-09 21:16 ` tip-bot for Josh Poimboeuf
  1 sibling, 0 replies; 14+ messages in thread
From: tip-bot for Josh Poimboeuf @ 2019-08-09 21:16 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, ndesaulniers, tglx, sedat.dilek, hpa, peterz,
	mingo, chris, jpoimboe

Commit-ID:  e6a9522ac3ff59980ea00e070b6b8573aface36a
Gitweb:     https://git.kernel.org/tip/e6a9522ac3ff59980ea00e070b6b8573aface36a
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Thu, 25 Jul 2019 15:29:57 -0500
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 9 Aug 2019 23:13:25 +0200

drm/i915: Remove redundant user_access_end() from __copy_from_user() error path

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>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://github.com/ClangBuiltLinux/linux/issues/617
Link: https://lkml.kernel.org/r/51a4155c5bc2ca847a9cbe85c1c11918bb193141.1564086017.git.jpoimboe@redhat.com

---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

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 @@ end:
 
 	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);
 	}

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path
  2019-08-09 20:19                     ` Sedat Dilek
@ 2019-08-10  5:59                       ` Thomas Gleixner
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Gleixner @ 2019-08-10  5:59 UTC (permalink / raw)
  To: Sedat Dilek
  Cc: Nick Desaulniers, Josh Poimboeuf, Chris Wilson, intel-gfx,
	dri-devel, LKML, Peter Zijlstra, Linus Torvalds,
	Nathan Chancellor, Arnd Bergmann

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 :)

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-08-10  5:59 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-25 20:29 [PATCH] drm/i915: Remove redundant user_access_end() from __copy_from_user() error path Josh Poimboeuf
2019-07-25 21:55 ` Thomas Gleixner
2019-07-26 19:05   ` Chris Wilson
2019-07-26 19:18     ` Thomas Gleixner
2019-07-26 19:30       ` Chris Wilson
2019-07-31 12:25         ` Sedat Dilek
2019-08-05 19:29           ` Sedat Dilek
2019-08-06 12:59             ` Josh Poimboeuf
2019-08-08 20:14               ` Nick Desaulniers
2019-08-08 20:21                 ` Thomas Gleixner
2019-08-08 23:02                   ` Nick Desaulniers
2019-08-09 20:19                     ` Sedat Dilek
2019-08-10  5:59                       ` Thomas Gleixner
2019-08-09 21:16 ` [tip:core/urgent] " tip-bot for Josh Poimboeuf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).