[PATCH] IO port bitmap cleanups, x86-64 oops fix

This patch brings x86-64 and i386 closer together, eliminating an oops
that LTP test ioperm02.c causes on x86-64.  An IO port permission bitmap
must be followed by an extra 0xff.

(Add comments to that effect, to avoid the problem in the future).

arch/i386/kernel/doublefault.c

@@ -50,8 +50,7 @@ struct tss_struct doublefault_tss __cacheline_aligned = {
 	.esp0		= STACK_START,
 	.ss0		= __KERNEL_DS,
 	.ldt		= 0,
-	.bitmap		= INVALID_IO_BITMAP_OFFSET,
-	.io_bitmap	= { [0 ... IO_BITMAP_SIZE ] = ~0 },
+	.io_bitmap_base	= INVALID_IO_BITMAP_OFFSET,
 
 	.eip		= (unsigned long) doublefault_fn,
 	.eflags		= 0x00000082,

arch/i386/kernel/ioport.c

@@ -17,17 +17,17 @@
 #include <linux/thread_info.h>
 
 /* Set EXTENT bits starting at BASE in BITMAP to value TURN_ON. */
-static void set_bitmap(unsigned long *bitmap, unsigned long base, unsigned long extent, int new_value)
+static void set_bitmap(unsigned long *bitmap, unsigned int base, unsigned int extent, int new_value)
 {
 	unsigned long mask;
 	unsigned long *bitmap_base = bitmap + (base / BITS_PER_LONG);
-	unsigned long low_index = base & (BITS_PER_LONG-1);
+	unsigned int low_index = base & (BITS_PER_LONG-1);
 	int length = low_index + extent;
 
 	if (low_index != 0) {
 		mask = (~0UL << low_index);
 		if (length < BITS_PER_LONG)
-				mask &= ~(~0UL << length);
+			mask &= ~(~0UL << length);
 		if (new_value)
 			*bitmap_base++ |= mask;
 		else
@@ -50,6 +50,7 @@ static void set_bitmap(unsigned long *bitmap, unsigned long base, unsigned long
 	}
 }
 
+
 /*
  * this changes the io permissions bitmap in the current task.
  */
@@ -57,10 +58,9 @@ asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
 {
 	struct thread_struct * t = &current->thread;
 	struct tss_struct * tss;
-	unsigned long *bitmap = NULL;
-	int ret = 0;
+	unsigned long *bitmap;
 
-	if ((from + num <= from) || (from + num > IO_BITMAP_SIZE*32))
+	if ((from + num <= from) || (from + num > IO_BITMAP_BITS))
 		return -EINVAL;
 	if (turn_on && !capable(CAP_SYS_RAWIO))
 		return -EPERM;
@@ -70,34 +70,28 @@ asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
 	 * IO bitmap up. ioperm() is much less timing critical than clone(),
 	 * this is why we delay this operation until now:
 	 */
-	if (!t->ts_io_bitmap) {
+	if (!t->io_bitmap_ptr) {
 		bitmap = kmalloc(IO_BITMAP_BYTES, GFP_KERNEL);
-		if (!bitmap) {
-			ret = -ENOMEM;
-			goto out;
-		}
+		if (!bitmap)
+			return -ENOMEM;
 
-		/*
-		 * just in case ...
-		 */
 		memset(bitmap, 0xff, IO_BITMAP_BYTES);
-		t->ts_io_bitmap = bitmap;
+		t->io_bitmap_ptr = bitmap;
 	}
 
 	/*
 	 * do it in the per-thread copy and in the TSS ...
 	 */
-	set_bitmap(t->ts_io_bitmap, from, num, !turn_on);
+	set_bitmap(t->io_bitmap_ptr, from, num, !turn_on);
 	tss = init_tss + get_cpu();
-	if (tss->bitmap == IO_BITMAP_OFFSET) { /* already active? */
+	if (tss->io_bitmap_base == IO_BITMAP_OFFSET) { /* already active? */
 		set_bitmap(tss->io_bitmap, from, num, !turn_on);
 	} else {
-		memcpy(tss->io_bitmap, t->ts_io_bitmap, IO_BITMAP_BYTES);
-		tss->bitmap = IO_BITMAP_OFFSET;	/* Activate it in the TSS */
+		memcpy(tss->io_bitmap, t->io_bitmap_ptr, IO_BITMAP_BYTES);
+		tss->io_bitmap_base = IO_BITMAP_OFFSET; /* Activate it in the TSS */
 	}
 	put_cpu();
-out:
-	return ret;
+	return 0;
 }
 
 /*

arch/i386/kernel/process.c

@@ -241,9 +241,9 @@ void exit_thread(void)
 	struct task_struct *tsk = current;
 
 	/* The process may have allocated an io port bitmap... nuke it. */
-	if (unlikely(NULL != tsk->thread.ts_io_bitmap)) {
-		kfree(tsk->thread.ts_io_bitmap);
-		tsk->thread.ts_io_bitmap = NULL;
+	if (unlikely(NULL != tsk->thread.io_bitmap_ptr)) {
+		kfree(tsk->thread.io_bitmap_ptr);
+		tsk->thread.io_bitmap_ptr = NULL;
 	}
 }
 
@@ -308,11 +308,11 @@ int copy_thread(int nr, unsigned long clone_flags, unsigned long esp,
 	savesegment(gs,p->thread.gs);
 
 	tsk = current;
-	if (unlikely(NULL != tsk->thread.ts_io_bitmap)) {
-		p->thread.ts_io_bitmap = kmalloc(IO_BITMAP_BYTES, GFP_KERNEL);
-		if (!p->thread.ts_io_bitmap)
+	if (unlikely(NULL != tsk->thread.io_bitmap_ptr)) {
+		p->thread.io_bitmap_ptr = kmalloc(IO_BITMAP_BYTES, GFP_KERNEL);
+		if (!p->thread.io_bitmap_ptr)
 			return -ENOMEM;
-		memcpy(p->thread.ts_io_bitmap, tsk->thread.ts_io_bitmap,
+		memcpy(p->thread.io_bitmap_ptr, tsk->thread.io_bitmap_ptr,
 			IO_BITMAP_BYTES);
 	}
 
@@ -342,8 +342,8 @@ int copy_thread(int nr, unsigned long clone_flags, unsigned long esp,
 
 	err = 0;
  out:
-	if (err && p->thread.ts_io_bitmap)
-		kfree(p->thread.ts_io_bitmap);
+	if (err && p->thread.io_bitmap_ptr)
+		kfree(p->thread.io_bitmap_ptr);
 	return err;
 }
 
@@ -492,8 +492,8 @@ struct task_struct * __switch_to(struct task_struct *prev_p, struct task_struct
 		loaddebug(next, 7);
 	}
 
-	if (unlikely(prev->ts_io_bitmap || next->ts_io_bitmap)) {
-		if (next->ts_io_bitmap) {
+	if (unlikely(prev->io_bitmap_ptr || next->io_bitmap_ptr)) {
+		if (next->io_bitmap_ptr) {
 			/*
 			 * 4 cachelines copy ... not good, but not that
 			 * bad either. Anyone got something better?
@@ -502,9 +502,9 @@ struct task_struct * __switch_to(struct task_struct *prev_p, struct task_struct
 			 * and playing VM tricks to switch the IO bitmap
 			 * is not really acceptable.]
 			 */
-			memcpy(tss->io_bitmap, next->ts_io_bitmap,
+			memcpy(tss->io_bitmap, next->io_bitmap_ptr,
 				IO_BITMAP_BYTES);
-			tss->bitmap = IO_BITMAP_OFFSET;
+			tss->io_bitmap_base = IO_BITMAP_OFFSET;
 		} else
 			/*
 			 * a bitmap offset pointing outside of the TSS limit
@@ -512,7 +512,7 @@ struct task_struct * __switch_to(struct task_struct *prev_p, struct task_struct
 			 * tries to use a port IO instruction. The first
 			 * sys_ioperm() call sets up the bitmap properly.
 			 */
-			tss->bitmap = INVALID_IO_BITMAP_OFFSET;
+			tss->io_bitmap_base = INVALID_IO_BITMAP_OFFSET;
 	}
 	return prev_p;
 }

arch/x86_64/kernel/ioport.c

@@ -18,11 +18,11 @@
 #include <asm/io.h>
 
 /* Set EXTENT bits starting at BASE in BITMAP to value TURN_ON. */
-static void set_bitmap(unsigned long *bitmap, short base, short extent, int new_value)
+static void set_bitmap(unsigned long *bitmap, unsigned int base, unsigned int extent, int new_value)
 {
 	unsigned long mask;
 	unsigned long *bitmap_base = bitmap + (base / BITS_PER_LONG);
-	unsigned long low_index = base & (BITS_PER_LONG-1);
+	unsigned int low_index = base & (BITS_PER_LONG-1);
 	int length = low_index + extent;
 
 	if (low_index != 0) {
@@ -58,9 +58,10 @@ static void set_bitmap(unsigned long *bitmap, short base, short extent, int new_
 asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
 {
 	struct thread_struct * t = &current->thread;
-	struct tss_struct *tss;
+	struct tss_struct * tss;
+	unsigned long *bitmap;
 
-	if ((from + num <= from) || (from + num > IO_BITMAP_SIZE*32))
+	if ((from + num <= from) || (from + num > IO_BITMAP_BITS))
 		return -EINVAL;
 	if (turn_on && !capable(CAP_SYS_RAWIO))
 		return -EPERM;
@@ -70,24 +71,25 @@ asmlinkage long sys_ioperm(unsigned long from, unsigned long num, int turn_on)
 	 * IO bitmap up. ioperm() is much less timing critical than clone(),
 	 * this is why we delay this operation until now:
 	 */
-	if (!t->io_bitmap_ptr) { 
-		t->io_bitmap_ptr = kmalloc(IO_BITMAP_BYTES, GFP_KERNEL);
-		if (!t->io_bitmap_ptr)
+	if (!t->io_bitmap_ptr) {
+		bitmap = kmalloc(IO_BITMAP_BYTES, GFP_KERNEL);
+		if (!bitmap)
 			return -ENOMEM;
 
-		memset(t->io_bitmap_ptr,0xff,IO_BITMAP_BYTES);
+		memset(bitmap, 0xff, IO_BITMAP_BYTES);
+		t->io_bitmap_ptr = bitmap;
 	}
 
 	/*
 	 * do it in the per-thread copy and in the TSS ...
 	 */
-	set_bitmap((unsigned long *) t->io_bitmap_ptr, from, num, !turn_on);
+	set_bitmap(t->io_bitmap_ptr, from, num, !turn_on);
 	tss = init_tss + get_cpu();
-	if (tss->io_map_base != IO_BITMAP_OFFSET) {
-		memcpy(tss->io_bitmap, t->io_bitmap_ptr, sizeof(tss->io_bitmap));
-		tss->io_map_base = IO_BITMAP_OFFSET;
+	if (tss->io_bitmap_base == IO_BITMAP_OFFSET) { /* already active? */
+		set_bitmap(tss->io_bitmap, from, num, !turn_on);
 	} else {
-		set_bitmap((unsigned long *) tss->io_bitmap, from, num, !turn_on);
+		memcpy(tss->io_bitmap, t->io_bitmap_ptr, IO_BITMAP_BYTES);
+		tss->io_bitmap_base = IO_BITMAP_OFFSET; /* Activate it in the TSS */
 	}
 	put_cpu();
 	return 0;

arch/x86_64/kernel/process.c

@@ -209,7 +209,7 @@ void exit_thread(void)
 	if (me->thread.io_bitmap_ptr) { 
 		kfree(me->thread.io_bitmap_ptr); 
 		me->thread.io_bitmap_ptr = NULL;
-		(init_tss + smp_processor_id())->io_map_base = 
+		(init_tss + smp_processor_id())->io_bitmap_base = 
 			INVALID_IO_BITMAP_OFFSET;
 	}
 }
@@ -312,11 +312,10 @@ int copy_thread(int nr, unsigned long clone_flags, unsigned long rsp,
 	asm("movl %%ds,%0" : "=m" (p->thread.ds));
 
 	if (unlikely(me->thread.io_bitmap_ptr != NULL)) { 
-		p->thread.io_bitmap_ptr = kmalloc((IO_BITMAP_SIZE+1)*4, GFP_KERNEL);
+		p->thread.io_bitmap_ptr = kmalloc(IO_BITMAP_BYTES, GFP_KERNEL);
 		if (!p->thread.io_bitmap_ptr) 
 			return -ENOMEM;
-		memcpy(p->thread.io_bitmap_ptr, me->thread.io_bitmap_ptr, 
-		       (IO_BITMAP_SIZE+1)*4);
+		memcpy(p->thread.io_bitmap_ptr, me->thread.io_bitmap_ptr, IO_BITMAP_BYTES);
 	} 
 
 	/*
@@ -449,9 +448,8 @@ struct task_struct *__switch_to(struct task_struct *prev_p, struct task_struct *
 			 * bad either. Anyone got something better?
 			 * This only affects processes which use ioperm().
 			 */
-			memcpy(tss->io_bitmap, next->io_bitmap_ptr,
-				 IO_BITMAP_SIZE*sizeof(u32));
-			tss->io_map_base = IO_BITMAP_OFFSET;
+			memcpy(tss->io_bitmap, next->io_bitmap_ptr, IO_BITMAP_BYTES);
+			tss->io_bitmap_base = IO_BITMAP_OFFSET;
 		} else {
 			/*
 			 * a bitmap offset pointing outside of the TSS limit
@@ -459,7 +457,7 @@ struct task_struct *__switch_to(struct task_struct *prev_p, struct task_struct *
 			 * tries to use a port IO instruction. The first
 			 * sys_ioperm() call sets up the bitmap properly.
 			 */
-			tss->io_map_base = INVALID_IO_BITMAP_OFFSET;
+			tss->io_bitmap_base = INVALID_IO_BITMAP_OFFSET;
 		}
 	}
 

arch/x86_64/kernel/setup64.c

@@ -288,7 +288,12 @@ void __init cpu_init (void)
 		estacks += EXCEPTION_STKSZ;
 	}
 
-	t->io_map_base = INVALID_IO_BITMAP_OFFSET;
+	t->io_bitmap_base = INVALID_IO_BITMAP_OFFSET;
+	/*
+	 * This is required because the CPU will access up to
+	 * 8 bits beyond the end of the IO permission bitmap.
+	 */
+	t->io_bitmap[IO_BITMAP_LONGS] = ~0UL;
 
 	atomic_inc(&init_mm.mm_count);
 	me->active_mm = &init_mm;

include/asm-i386/processor.h

@@ -291,10 +291,11 @@ extern unsigned int mca_pentium_flag;
 #define TASK_UNMAPPED_BASE	(PAGE_ALIGN(TASK_SIZE / 3))
 
 /*
- * Size of io_bitmap in longwords: 32 is ports 0-0x3ff.
+ * Size of io_bitmap, covering ports 0 to 0x3ff.
  */
-#define IO_BITMAP_SIZE	32
-#define IO_BITMAP_BYTES	(IO_BITMAP_SIZE * 4)
+#define IO_BITMAP_BITS  1024
+#define IO_BITMAP_BYTES (IO_BITMAP_BITS/8)
+#define IO_BITMAP_LONGS (IO_BITMAP_BYTES/sizeof(long))
 #define IO_BITMAP_OFFSET offsetof(struct tss_struct,io_bitmap)
 #define INVALID_IO_BITMAP_OFFSET 0x8000
 
@@ -373,8 +374,14 @@ struct tss_struct {
 	unsigned short	fs, __fsh;
 	unsigned short	gs, __gsh;
 	unsigned short	ldt, __ldth;
-	unsigned short	trace, bitmap;
-	unsigned long	io_bitmap[IO_BITMAP_SIZE+1];
+	unsigned short	trace, io_bitmap_base;
+	/*
+	 * The extra 1 is there because the CPU will access an
+	 * additional byte beyond the end of the IO permission
+	 * bitmap. The extra byte must be all 1 bits, and must
+	 * be within the limit.
+	 */
+	unsigned long	io_bitmap[IO_BITMAP_LONGS + 1];
 	/*
 	 * pads the TSS to be cacheline-aligned (size is 0x100)
 	 */
@@ -383,7 +390,7 @@ struct tss_struct {
 	 * .. and then another 0x100 bytes for emergency kernel stack
 	 */
 	unsigned long stack[64];
-};
+} __attribute__((packed));
 
 struct thread_struct {
 /* cached TLS descriptors. */
@@ -405,22 +412,28 @@ struct thread_struct {
 	unsigned long		v86flags, v86mask, saved_esp0;
 	unsigned int		saved_fs, saved_gs;
 /* IO permissions */
-	unsigned long	*ts_io_bitmap;
+	unsigned long	*io_bitmap_ptr;
 };
 
 #define INIT_THREAD  {							\
 	.vm86_info = NULL,						\
-	.ts_io_bitmap = NULL,						\
+	.io_bitmap_ptr = NULL,						\
 }
 
+/*
+ * Note that the .io_bitmap member must be extra-big. This is because
+ * the CPU will access an additional byte beyond the end of the IO
+ * permission bitmap. The extra byte must be all 1 bits, and must
+ * be within the limit.
+ */
 #define INIT_TSS  {							\
 	.esp0		= sizeof(init_stack) + (long)&init_stack,	\
 	.ss0		= __KERNEL_DS,					\
 	.esp1		= sizeof(init_tss[0]) + (long)&init_tss[0],	\
 	.ss1		= __KERNEL_CS,					\
 	.ldt		= GDT_ENTRY_LDT,				\
-	.bitmap		= INVALID_IO_BITMAP_OFFSET,			\
-	.io_bitmap	= { [ 0 ... IO_BITMAP_SIZE ] = ~0 },		\
+	.io_bitmap_base	= INVALID_IO_BITMAP_OFFSET,			\
+	.io_bitmap	= { [ 0 ... IO_BITMAP_LONGS] = ~0 },		\
 }
 
 static inline void load_esp0(struct tss_struct *tss, unsigned long esp0)

include/asm-x86_64/processor.h

@@ -178,10 +178,11 @@ static inline void clear_in_cr4 (unsigned long mask)
 	(test_thread_flag(TIF_IA32) ? TASK_UNMAPPED_32 : TASK_UNMAPPED_64)  
 
 /*
- * Size of io_bitmap in longwords: 32 is ports 0-0x3ff.
+ * Size of io_bitmap, covering ports 0 to 0x3ff.
  */
-#define IO_BITMAP_SIZE	32
-#define IO_BITMAP_BYTES (IO_BITMAP_SIZE * 4)
+#define IO_BITMAP_BITS  1024
+#define IO_BITMAP_BYTES (IO_BITMAP_BITS/8)
+#define IO_BITMAP_LONGS (IO_BITMAP_BYTES/sizeof(long))
 #define IO_BITMAP_OFFSET offsetof(struct tss_struct,io_bitmap)
 #define INVALID_IO_BITMAP_OFFSET 0x8000
 
@@ -213,8 +214,18 @@ struct tss_struct {
 	u32 reserved3;
 	u32 reserved4;
 	u16 reserved5;
-	u16 io_map_base;
-	u32 io_bitmap[IO_BITMAP_SIZE];
+	u16 io_bitmap_base;
+	/*
+	 * The extra 1 is there because the CPU will access an
+	 * additional byte beyond the end of the IO permission
+	 * bitmap. The extra byte must be all 1 bits, and must
+	 * be within the limit. Thus we have:
+	 *
+	 * 128 bytes, the bitmap itself, for ports 0..0x3ff
+	 * 8 bytes, for an extra "long" of ~0UL
+	 */
+	unsigned long io_bitmap[IO_BITMAP_LONGS + 1];
+	u32 __cacheline_filler[4];      /* size is 0x100 */
 } __attribute__((packed)) ____cacheline_aligned;
 
 struct thread_struct {
@@ -238,7 +249,7 @@ struct thread_struct {
 /* IO permissions. the bitmap could be moved into the GDT, that would make
    switch faster for a limited number of ioperm using tasks. -AK */
 	int		ioperm;
-	u32	*io_bitmap_ptr;
+	unsigned long	*io_bitmap_ptr;
 /* cached TLS descriptors. */
 	u64 tls_array[GDT_ENTRY_TLS_ENTRIES];
 };