x86/ldt: Rename ldr_struct.size to .n_entries

PATCH In review Borislav Petkov 4 participants 5 messages Jun 6, 2017 – Jun 8, 2017

Subsystems (2)

x86 perf events

lore

Share & tools

lore

Borislav Petkov

author maintainer

2017-06-06 17:31 UTC #0 lore raw

PATCH 1/1 x86/ldt: Rename ldr_struct.size to .n_entries

From: Borislav Petkov <[email protected]>

... because this is exactly what it is: the number of entries in the LDT. Calling it "size" is simply confusing and it is actually begging to be called "n_entries" or somesuch, especially if you see constructs like:

	alloc_size = size * LDT_ENTRY_SIZE;

since LDT_ENTRY_SIZE is the size of a single entry.

There should be no functionality change resulting from this patch, as the before/after output from tools/testing/selftests/x86/ldt_gdt.c shows.

Signed-off-by Borislav Petkov <[email protected]>

---
 arch/x86/events/core.c             |  2 +-
 arch/x86/include/asm/mmu_context.h |  4 ++--
 arch/x86/kernel/ldt.c              | 49 +++++++++++++++++++-------------------
 arch/x86/kernel/process_64.c       |  2 +-
 arch/x86/kernel/step.c             |  2 +-
 arch/x86/math-emu/fpu_system.h     |  2 +-
 6 files changed, 31 insertions(+), 30 deletions(-)

arch/x86/events/core.c +1 -1

index 400bea15f97e..c12b72f6972a 100644
@@ -2342,7 +2342,7 @@ static unsigned long get_segment_base(unsigned int segment)
 
 		/* IRQs are off, so this synchronizes with smp_store_release */
 		ldt = lockless_dereference(current->active_mm->context.ldt);
-		if (!ldt || idx > ldt->size)
+		if (!ldt || idx > ldt->n_entries)
 			return 0;
 
 		desc = &ldt->entries[idx];

arch/x86/include/asm/mmu_context.h +2 -2

index f20d7ea47095..435b43f77ab3 100644
@@ -47,7 +47,7 @@ struct ldt_struct {
 	 * allocations, but it's not worth trying to optimize.
 	 */
 	struct desc_struct *entries;
-	unsigned int size;
+	unsigned int n_entries;
 };
 
 /*

@@ -87,7 +87,7 @@ static inline void load_mm_ldt(struct mm_struct *mm)
 	 */
 
 	if (unlikely(ldt))
-		set_ldt(ldt->entries, ldt->size);
+		set_ldt(ldt->entries, ldt->n_entries);
 	else
 		clear_LDT();
 #else

arch/x86/kernel/ldt.c +25 ~~-24~~

135 lines not on this page · raw message

arch/x86/kernel/process_64.c +1 -1

index b6840bf3940b..95c76edc4587 100644
@@ -142,7 +142,7 @@ void release_thread(struct task_struct *dead_task)
 			pr_warn("WARNING: dead process %s still has LDT? <%p/%d>\n",
 				dead_task->comm,
 				dead_task->mm->context.ldt->entries,
-				dead_task->mm->context.ldt->size);
+				dead_task->mm->context.ldt->n_entries);
 			BUG();
 		}
 #endif

arch/x86/kernel/step.c +1 -1

index f07f83b3611b..59b8b9470ed6 100644
@@ -34,7 +34,7 @@ unsigned long convert_ip_to_linear(struct task_struct *child, struct pt_regs *re
 
 		mutex_lock(&child->mm->context.lock);
 		if (unlikely(!child->mm->context.ldt ||
-			     seg >= child->mm->context.ldt->size))
+			     seg >= child->mm->context.ldt->n_entries))
 			addr = -1L; /* bogus selector, access would fault */
 		else {
 			desc = &child->mm->context.ldt->entries[seg];

arch/x86/math-emu/fpu_system.h +1 -1

index 5e044d506b7a..8252636e8136 100644
@@ -27,7 +27,7 @@ static inline struct desc_struct FPU_get_ldt_descriptor(unsigned seg)
 #ifdef CONFIG_MODIFY_LDT_SYSCALL
 	seg >>= 3;
 	mutex_lock(&current->mm->context.lock);
-	if (current->mm->context.ldt && seg < current->mm->context.ldt->size)
+	if (current->mm->context.ldt && seg < current->mm->context.ldt->n_entries)
 		ret = current->mm->context.ldt->entries[seg];
 	mutex_unlock(&current->mm->context.lock);
 #endif

Signature · 1 line

2.13.0

Andy Lutomirski

2017-06-06 22:20 UTC #1 lore raw

Quoted 6 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>
... because this is exactly what it is: the number of entries in the LDT. Calling it "size" is simply confusing and it is actually begging to be called "n_entries" or somesuch, especially if you see constructs like:

In the subject: ldt_struct, perhaps?

Otherwise looks good.

Borislav Petkov

author maintainer

2017-06-07 10:24 UTC #2 lore raw

Quoted 1 line · Andy Lutomirski

In the subject: ldt_struct, perhaps?

Right. Committer, pls fix. :-)

Quoted 1 line

Otherwise looks good.

Thanks.

Signature · 4 lines

Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Ingo Molnar

2017-06-08 07:27 UTC #3 lore raw

Quoted 4 lines · Borislav Petkov

Quoted 1 line · Andy Lutomirski
In the subject: ldt_struct, perhaps?
Right. Committer, pls fix. :-)

Fixed. I also renamed 'n_entries' to 'nr_entries', which is the canonical form:

triton:~/tip> git grep -w n_entries | wc -l
191

triton:~/tip> git grep -w nr_entries | wc -l
743

If we do a rename, we should do it right, painted bright red! ;-)

Thanks,

Ingo

tip-bot for Borislav Petkov

merge bot

2017-06-08 09:24 UTC #4 lore raw

[tip:x86/mm] x86/ldt: Rename ldt_struct::size to ::nr_entries

In reply to Borislav Petkov

x86/ldt: Rename ldt_struct::size to ::nr_entries

Commit-ID:  bbf79d21bd4627a01ca8721c9373752819f8e4cc
Gitweb:     http://git.kernel.org/tip/bbf79d21bd4627a01ca8721c9373752819f8e4cc
Author:     Borislav Petkov <[email protected]>
AuthorDate: Tue, 6 Jun 2017 19:31:16 +0200
Committer:  Ingo Molnar <[email protected]>
CommitDate: Thu, 8 Jun 2017 09:28:21 +0200

x86/ldt: Rename ldt_struct::size to ::nr_entries

... because this is exactly what it is: the number of entries in the LDT. Calling it "size" is simply confusing and it is actually begging to be called "nr_entries" or somesuch, especially if you see constructs like:

	alloc_size = size * LDT_ENTRY_SIZE;

since LDT_ENTRY_SIZE is the size of a single entry.

There should be no functionality change resulting from this patch, as the before/after output from tools/testing/selftests/x86/ldt_gdt.c shows.

Signed-off-by Borislav Petkov <[email protected]>

Acked-by Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

[ Renamed 'n_entries' to 'nr_entries' ]

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/events/core.c             |  2 +-
 arch/x86/include/asm/mmu_context.h |  4 ++--
 arch/x86/kernel/ldt.c              | 49 +++++++++++++++++++-------------------
 arch/x86/kernel/process_64.c       |  2 +-
 arch/x86/kernel/step.c             |  2 +-
 arch/x86/math-emu/fpu_system.h     |  2 +-
 6 files changed, 31 insertions(+), 30 deletions(-)