All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/ldt: Off by one in get_segment_base()
@ 2017-08-18 10:30 ` Dan Carpenter
  0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2017-08-18 10:30 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin,
	Thomas Gleixner, H. Peter Anvin, x86, linux-kernel,
	kernel-janitors

ldt->entries[] is allocated in alloc_ldt_struct().  It has
ldt->nr_entries elements and ldt->nr_entries is capped at LDT_ENTRIES.
So if "idx" is == ldt->nr_entries then we're reading beyond the end of
the buffer.  It seems duplicative to have two limit checks when one
would work just as well so I removed the check against LDT_ENTRIES.

The gdt_page.gdt[] array has GDT_ENTRIES entries.

Fixes: d07bdfd322d3 ("perf/x86: Fix USER/KERNEL tagging of samples properly")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This is a static checker fix.  I'm pretty certain it's correct.  But I
don't understand the code terribly well, so please let me know and I'll
be happy to send a v2 if needed.

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index af12e294caed..939050169d12 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2335,12 +2335,9 @@ static unsigned long get_segment_base(unsigned int segment)
 #ifdef CONFIG_MODIFY_LDT_SYSCALL
 		struct ldt_struct *ldt;
 
-		if (idx > LDT_ENTRIES)
-			return 0;
-
 		/* IRQs are off, so this synchronizes with smp_store_release */
 		ldt = lockless_dereference(current->active_mm->context.ldt);
-		if (!ldt || idx > ldt->nr_entries)
+		if (!ldt || idx >= ldt->nr_entries)
 			return 0;
 
 		desc = &ldt->entries[idx];
@@ -2348,7 +2345,7 @@ static unsigned long get_segment_base(unsigned int segment)
 		return 0;
 #endif
 	} else {
-		if (idx > GDT_ENTRIES)
+		if (idx >= GDT_ENTRIES)
 			return 0;
 
 		desc = raw_cpu_ptr(gdt_page.gdt) + idx;

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

* [PATCH] x86/ldt: Off by one in get_segment_base()
@ 2017-08-18 10:30 ` Dan Carpenter
  0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2017-08-18 10:30 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Alexander Shishkin,
	Thomas Gleixner, H. Peter Anvin, x86, linux-kernel,
	kernel-janitors

ldt->entries[] is allocated in alloc_ldt_struct().  It has
ldt->nr_entries elements and ldt->nr_entries is capped at LDT_ENTRIES.
So if "idx" is = ldt->nr_entries then we're reading beyond the end of
the buffer.  It seems duplicative to have two limit checks when one
would work just as well so I removed the check against LDT_ENTRIES.

The gdt_page.gdt[] array has GDT_ENTRIES entries.

Fixes: d07bdfd322d3 ("perf/x86: Fix USER/KERNEL tagging of samples properly")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This is a static checker fix.  I'm pretty certain it's correct.  But I
don't understand the code terribly well, so please let me know and I'll
be happy to send a v2 if needed.

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index af12e294caed..939050169d12 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2335,12 +2335,9 @@ static unsigned long get_segment_base(unsigned int segment)
 #ifdef CONFIG_MODIFY_LDT_SYSCALL
 		struct ldt_struct *ldt;
 
-		if (idx > LDT_ENTRIES)
-			return 0;
-
 		/* IRQs are off, so this synchronizes with smp_store_release */
 		ldt = lockless_dereference(current->active_mm->context.ldt);
-		if (!ldt || idx > ldt->nr_entries)
+		if (!ldt || idx >= ldt->nr_entries)
 			return 0;
 
 		desc = &ldt->entries[idx];
@@ -2348,7 +2345,7 @@ static unsigned long get_segment_base(unsigned int segment)
 		return 0;
 #endif
 	} else {
-		if (idx > GDT_ENTRIES)
+		if (idx >= GDT_ENTRIES)
 			return 0;
 
 		desc = raw_cpu_ptr(gdt_page.gdt) + idx;

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

* [tip:x86/urgent] x86/ldt: Fix off by one in get_segment_base()
  2017-08-18 10:30 ` Dan Carpenter
  (?)
@ 2017-08-29 11:11 ` tip-bot for Dan Carpenter
  -1 siblings, 0 replies; 3+ messages in thread
From: tip-bot for Dan Carpenter @ 2017-08-29 11:11 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, linux-kernel, alexander.shishkin, mingo, hpa, acme, tglx,
	dan.carpenter, torvalds, luto

Commit-ID:  eaa2f87c6b840b83827c40db6eb8481689570259
Gitweb:     http://git.kernel.org/tip/eaa2f87c6b840b83827c40db6eb8481689570259
Author:     Dan Carpenter <dan.carpenter@oracle.com>
AuthorDate: Fri, 18 Aug 2017 13:30:30 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 29 Aug 2017 11:55:15 +0200

x86/ldt: Fix off by one in get_segment_base()

ldt->entries[] is allocated in alloc_ldt_struct().  It has
ldt->nr_entries elements and ldt->nr_entries is capped at LDT_ENTRIES.
So if "idx" is == ldt->nr_entries then we're reading beyond the end of
the buffer.  It seems duplicative to have two limit checks when one
would work just as well so I removed the check against LDT_ENTRIES.

The gdt_page.gdt[] array has GDT_ENTRIES entries.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Andy Lutomirski <luto@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: kernel-janitors@vger.kernel.org
Fixes: d07bdfd322d3 ("perf/x86: Fix USER/KERNEL tagging of samples properly")
Link: http://lkml.kernel.org/r/20170818102516.gqwm4xdvvuvjw5ho@mwanda
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/events/core.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index af12e29..9390501 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2335,12 +2335,9 @@ static unsigned long get_segment_base(unsigned int segment)
 #ifdef CONFIG_MODIFY_LDT_SYSCALL
 		struct ldt_struct *ldt;
 
-		if (idx > LDT_ENTRIES)
-			return 0;
-
 		/* IRQs are off, so this synchronizes with smp_store_release */
 		ldt = lockless_dereference(current->active_mm->context.ldt);
-		if (!ldt || idx > ldt->nr_entries)
+		if (!ldt || idx >= ldt->nr_entries)
 			return 0;
 
 		desc = &ldt->entries[idx];
@@ -2348,7 +2345,7 @@ static unsigned long get_segment_base(unsigned int segment)
 		return 0;
 #endif
 	} else {
-		if (idx > GDT_ENTRIES)
+		if (idx >= GDT_ENTRIES)
 			return 0;
 
 		desc = raw_cpu_ptr(gdt_page.gdt) + idx;

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

end of thread, other threads:[~2017-08-29 11:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-18 10:30 [PATCH] x86/ldt: Off by one in get_segment_base() Dan Carpenter
2017-08-18 10:30 ` Dan Carpenter
2017-08-29 11:11 ` [tip:x86/urgent] x86/ldt: Fix off " tip-bot for Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.