subject:"\[PATCH v12 12\/22\] selftests\/vm\: generic cleanup"

Re: [PATCH v12 12/22] selftests/vm: generic cleanup

2018-03-16 Thread Dave Hansen

On 02/21/2018 05:55 PM, Ram Pai wrote:
> cleanup the code to satisfy coding styles.
> 
> cc: Dave Hansen 
> cc: Florian Weimer 
> Signed-off-by: Ram Pai 
> ---
>  tools/testing/selftests/vm/protection_keys.c |   81 
> ++
>  1 files changed, 43 insertions(+), 38 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/protection_keys.c 
> b/tools/testing/selftests/vm/protection_keys.c
> index 6054093..6fdd8f5 100644
> --- a/tools/testing/selftests/vm/protection_keys.c
> +++ b/tools/testing/selftests/vm/protection_keys.c
> @@ -4,7 +4,7 @@
>   *
>   * There are examples in here of:
>   *  * how to set protection keys on memory
> - *  * how to set/clear bits in pkey registers (the rights register)
> + *  * how to set/clear bits in Protection Key registers (the rights register)

I don't think CodingStyle says to do this. :)

>   *  * how to handle SEGV_PKUERR signals and extract pkey-relevant
>   *information from the siginfo
>   *
> @@ -13,13 +13,18 @@
>   *   prefault pages in at malloc, or not
>   *   protect MPX bounds tables with protection keys?
>   *   make sure VMA splitting/merging is working correctly
> - *   OOMs can destroy mm->mmap (see exit_mmap()), so make sure it is immune 
> to pkeys
> - *   look for pkey "leaks" where it is still set on a VMA but "freed" back 
> to the kernel
> - *   do a plain mprotect() to a mprotect_pkey() area and make sure the pkey 
> sticks
> + *   OOMs can destroy mm->mmap (see exit_mmap()),
> + *   so make sure it is immune to pkeys
> + *   look for pkey "leaks" where it is still set on a VMA
> + *but "freed" back to the kernel
> + *   do a plain mprotect() to a mprotect_pkey() area and make
> + *sure the pkey sticks

Ram, I'm not sure where this came from, but this looks horrid.  Please
don't do this to the file

>   * Compile like this:
> - *   gcc  -o protection_keys-O2 -g -std=gnu99 -pthread -Wall 
> protection_keys.c -lrt -ldl -lm
> - *   gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall 
> protection_keys.c -lrt -ldl -lm
> + *   gcc  -o protection_keys-O2 -g -std=gnu99
> + *-pthread -Wall protection_keys.c -lrt -ldl -lm
> + *   gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99
> + *-pthread -Wall protection_keys.c -lrt -ldl -lm
>   */

Please just leave this, or remove it from the file.  It was a long line
so it could be copied and pasted, this ruins that.



>  #define _GNU_SOURCE
>  #include 
> @@ -251,26 +256,11 @@ void signal_handler(int signum, siginfo_t *si, void 
> *vucontext)
>   dprintf1("signal pkey_reg from  pkey_reg: %016lx\n", __rdpkey_reg());
>   dprintf1("pkey from siginfo: %jx\n", siginfo_pkey);
>   *(u64 *)pkey_reg_ptr = 0x;
> - dprintf1("WARNING: set PRKU=0 to allow faulting instruction to 
> continue\n");
> + dprintf1("WARNING: set PKEY_REG=0 to allow faulting instruction "
> + "to continue\n");
>   pkey_faults++;
>   dprintf1("==\n");
>   return;
> - if (trapno == 14) {
> - fprintf(stderr,
> - "ERROR: In signal handler, page fault, trapno = %d, ip 
> = %016lx\n",
> - trapno, ip);
> - fprintf(stderr, "si_addr %p\n", si->si_addr);
> - fprintf(stderr, "REG_ERR: %lx\n",
> - (unsigned 
> long)uctxt->uc_mcontext.gregs[REG_ERR]);
> - exit(1);
> - } else {
> - fprintf(stderr, "unexpected trap %d! at 0x%lx\n", trapno, ip);
> - fprintf(stderr, "si_addr %p\n", si->si_addr);
> - fprintf(stderr, "REG_ERR: %lx\n",
> - (unsigned 
> long)uctxt->uc_mcontext.gregs[REG_ERR]);
> - exit(2);
> - }
> - dprint_in_signal = 0;
>  }

I think this is just randomly removing code now.

I think you should probably just drop this patch.  It's not really
brining anything useful.

Re: [PATCH v12 12/22] selftests/vm: generic cleanup

2018-03-16 Thread Dave Hansen

On 02/21/2018 05:55 PM, Ram Pai wrote:
> cleanup the code to satisfy coding styles.
> 
> cc: Dave Hansen 
> cc: Florian Weimer 
> Signed-off-by: Ram Pai 
> ---
>  tools/testing/selftests/vm/protection_keys.c |   81 
> ++
>  1 files changed, 43 insertions(+), 38 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/protection_keys.c 
> b/tools/testing/selftests/vm/protection_keys.c
> index 6054093..6fdd8f5 100644
> --- a/tools/testing/selftests/vm/protection_keys.c
> +++ b/tools/testing/selftests/vm/protection_keys.c
> @@ -4,7 +4,7 @@
>   *
>   * There are examples in here of:
>   *  * how to set protection keys on memory
> - *  * how to set/clear bits in pkey registers (the rights register)
> + *  * how to set/clear bits in Protection Key registers (the rights register)

I don't think CodingStyle says to do this. :)

>   *  * how to handle SEGV_PKUERR signals and extract pkey-relevant
>   *information from the siginfo
>   *
> @@ -13,13 +13,18 @@
>   *   prefault pages in at malloc, or not
>   *   protect MPX bounds tables with protection keys?
>   *   make sure VMA splitting/merging is working correctly
> - *   OOMs can destroy mm->mmap (see exit_mmap()), so make sure it is immune 
> to pkeys
> - *   look for pkey "leaks" where it is still set on a VMA but "freed" back 
> to the kernel
> - *   do a plain mprotect() to a mprotect_pkey() area and make sure the pkey 
> sticks
> + *   OOMs can destroy mm->mmap (see exit_mmap()),
> + *   so make sure it is immune to pkeys
> + *   look for pkey "leaks" where it is still set on a VMA
> + *but "freed" back to the kernel
> + *   do a plain mprotect() to a mprotect_pkey() area and make
> + *sure the pkey sticks

Ram, I'm not sure where this came from, but this looks horrid.  Please
don't do this to the file

>   * Compile like this:
> - *   gcc  -o protection_keys-O2 -g -std=gnu99 -pthread -Wall 
> protection_keys.c -lrt -ldl -lm
> - *   gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall 
> protection_keys.c -lrt -ldl -lm
> + *   gcc  -o protection_keys-O2 -g -std=gnu99
> + *-pthread -Wall protection_keys.c -lrt -ldl -lm
> + *   gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99
> + *-pthread -Wall protection_keys.c -lrt -ldl -lm
>   */

Please just leave this, or remove it from the file.  It was a long line
so it could be copied and pasted, this ruins that.



>  #define _GNU_SOURCE
>  #include 
> @@ -251,26 +256,11 @@ void signal_handler(int signum, siginfo_t *si, void 
> *vucontext)
>   dprintf1("signal pkey_reg from  pkey_reg: %016lx\n", __rdpkey_reg());
>   dprintf1("pkey from siginfo: %jx\n", siginfo_pkey);
>   *(u64 *)pkey_reg_ptr = 0x;
> - dprintf1("WARNING: set PRKU=0 to allow faulting instruction to 
> continue\n");
> + dprintf1("WARNING: set PKEY_REG=0 to allow faulting instruction "
> + "to continue\n");
>   pkey_faults++;
>   dprintf1("==\n");
>   return;
> - if (trapno == 14) {
> - fprintf(stderr,
> - "ERROR: In signal handler, page fault, trapno = %d, ip 
> = %016lx\n",
> - trapno, ip);
> - fprintf(stderr, "si_addr %p\n", si->si_addr);
> - fprintf(stderr, "REG_ERR: %lx\n",
> - (unsigned 
> long)uctxt->uc_mcontext.gregs[REG_ERR]);
> - exit(1);
> - } else {
> - fprintf(stderr, "unexpected trap %d! at 0x%lx\n", trapno, ip);
> - fprintf(stderr, "si_addr %p\n", si->si_addr);
> - fprintf(stderr, "REG_ERR: %lx\n",
> - (unsigned 
> long)uctxt->uc_mcontext.gregs[REG_ERR]);
> - exit(2);
> - }
> - dprint_in_signal = 0;
>  }

I think this is just randomly removing code now.

I think you should probably just drop this patch.  It's not really
brining anything useful.

[PATCH v12 12/22] selftests/vm: generic cleanup

2018-02-21 Thread Ram Pai

cleanup the code to satisfy coding styles.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |   81 ++
 1 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index 6054093..6fdd8f5 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -4,7 +4,7 @@
  *
  * There are examples in here of:
  *  * how to set protection keys on memory
- *  * how to set/clear bits in pkey registers (the rights register)
+ *  * how to set/clear bits in Protection Key registers (the rights register)
  *  * how to handle SEGV_PKUERR signals and extract pkey-relevant
  *information from the siginfo
  *
@@ -13,13 +13,18 @@
  * prefault pages in at malloc, or not
  * protect MPX bounds tables with protection keys?
  * make sure VMA splitting/merging is working correctly
- * OOMs can destroy mm->mmap (see exit_mmap()), so make sure it is immune 
to pkeys
- * look for pkey "leaks" where it is still set on a VMA but "freed" back 
to the kernel
- * do a plain mprotect() to a mprotect_pkey() area and make sure the pkey 
sticks
+ * OOMs can destroy mm->mmap (see exit_mmap()),
+ * so make sure it is immune to pkeys
+ * look for pkey "leaks" where it is still set on a VMA
+ *  but "freed" back to the kernel
+ * do a plain mprotect() to a mprotect_pkey() area and make
+ *  sure the pkey sticks
  *
  * Compile like this:
- * gcc  -o protection_keys-O2 -g -std=gnu99 -pthread -Wall 
protection_keys.c -lrt -ldl -lm
- * gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall 
protection_keys.c -lrt -ldl -lm
+ * gcc  -o protection_keys-O2 -g -std=gnu99
+ *  -pthread -Wall protection_keys.c -lrt -ldl -lm
+ * gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99
+ *  -pthread -Wall protection_keys.c -lrt -ldl -lm
  */
 #define _GNU_SOURCE
 #include 
@@ -251,26 +256,11 @@ void signal_handler(int signum, siginfo_t *si, void 
*vucontext)
dprintf1("signal pkey_reg from  pkey_reg: %016lx\n", __rdpkey_reg());
dprintf1("pkey from siginfo: %jx\n", siginfo_pkey);
*(u64 *)pkey_reg_ptr = 0x;
-   dprintf1("WARNING: set PRKU=0 to allow faulting instruction to 
continue\n");
+   dprintf1("WARNING: set PKEY_REG=0 to allow faulting instruction "
+   "to continue\n");
pkey_faults++;
dprintf1("==\n");
return;
-   if (trapno == 14) {
-   fprintf(stderr,
-   "ERROR: In signal handler, page fault, trapno = %d, ip 
= %016lx\n",
-   trapno, ip);
-   fprintf(stderr, "si_addr %p\n", si->si_addr);
-   fprintf(stderr, "REG_ERR: %lx\n",
-   (unsigned 
long)uctxt->uc_mcontext.gregs[REG_ERR]);
-   exit(1);
-   } else {
-   fprintf(stderr, "unexpected trap %d! at 0x%lx\n", trapno, ip);
-   fprintf(stderr, "si_addr %p\n", si->si_addr);
-   fprintf(stderr, "REG_ERR: %lx\n",
-   (unsigned 
long)uctxt->uc_mcontext.gregs[REG_ERR]);
-   exit(2);
-   }
-   dprint_in_signal = 0;
 }
 
 int wait_all_children(void)
@@ -415,7 +405,7 @@ void pkey_disable_set(int pkey, int flags)
 {
unsigned long syscall_flags = 0;
int ret;
-   int pkey_rights;
+   u32 pkey_rights;
pkey_reg_t orig_pkey_reg = rdpkey_reg();
 
dprintf1("START->%s(%d, 0x%x)\n", __func__,
@@ -453,7 +443,7 @@ void pkey_disable_clear(int pkey, int flags)
 {
unsigned long syscall_flags = 0;
int ret;
-   int pkey_rights = pkey_get(pkey, syscall_flags);
+   u32 pkey_rights = pkey_get(pkey, syscall_flags);
pkey_reg_t orig_pkey_reg = rdpkey_reg();
 
pkey_assert(flags & (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE));
@@ -516,9 +506,10 @@ int sys_mprotect_pkey(void *ptr, size_t size, unsigned 
long orig_prot,
return sret;
 }
 
-int sys_pkey_alloc(unsigned long flags, unsigned long init_val)
+int sys_pkey_alloc(unsigned long flags, u64 init_val)
 {
int ret = syscall(SYS_pkey_alloc, flags, init_val);
+
dprintf1("%s(flags=%lx, init_val=%lx) syscall ret: %d errno: %d\n",
__func__, flags, init_val, ret, errno);
return ret;
@@ -542,7 +533,7 @@ void pkey_set_shadow(u32 key, u64 init_val)
 int alloc_pkey(void)
 {
int ret;
-   unsigned long init_val = 0x0;
+   u64 init_val = 0x0;
 
dprintf1("%s()::%d, pkey_reg: 0x%016lx shadow: %016lx\n", __func__,
__LINE__,

[PATCH v12 12/22] selftests/vm: generic cleanup

2018-02-21 Thread Ram Pai

cleanup the code to satisfy coding styles.

cc: Dave Hansen 
cc: Florian Weimer 
Signed-off-by: Ram Pai 
---
 tools/testing/selftests/vm/protection_keys.c |   81 ++
 1 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/tools/testing/selftests/vm/protection_keys.c 
b/tools/testing/selftests/vm/protection_keys.c
index 6054093..6fdd8f5 100644
--- a/tools/testing/selftests/vm/protection_keys.c
+++ b/tools/testing/selftests/vm/protection_keys.c
@@ -4,7 +4,7 @@
  *
  * There are examples in here of:
  *  * how to set protection keys on memory
- *  * how to set/clear bits in pkey registers (the rights register)
+ *  * how to set/clear bits in Protection Key registers (the rights register)
  *  * how to handle SEGV_PKUERR signals and extract pkey-relevant
  *information from the siginfo
  *
@@ -13,13 +13,18 @@
  * prefault pages in at malloc, or not
  * protect MPX bounds tables with protection keys?
  * make sure VMA splitting/merging is working correctly
- * OOMs can destroy mm->mmap (see exit_mmap()), so make sure it is immune 
to pkeys
- * look for pkey "leaks" where it is still set on a VMA but "freed" back 
to the kernel
- * do a plain mprotect() to a mprotect_pkey() area and make sure the pkey 
sticks
+ * OOMs can destroy mm->mmap (see exit_mmap()),
+ * so make sure it is immune to pkeys
+ * look for pkey "leaks" where it is still set on a VMA
+ *  but "freed" back to the kernel
+ * do a plain mprotect() to a mprotect_pkey() area and make
+ *  sure the pkey sticks
  *
  * Compile like this:
- * gcc  -o protection_keys-O2 -g -std=gnu99 -pthread -Wall 
protection_keys.c -lrt -ldl -lm
- * gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall 
protection_keys.c -lrt -ldl -lm
+ * gcc  -o protection_keys-O2 -g -std=gnu99
+ *  -pthread -Wall protection_keys.c -lrt -ldl -lm
+ * gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99
+ *  -pthread -Wall protection_keys.c -lrt -ldl -lm
  */
 #define _GNU_SOURCE
 #include 
@@ -251,26 +256,11 @@ void signal_handler(int signum, siginfo_t *si, void 
*vucontext)
dprintf1("signal pkey_reg from  pkey_reg: %016lx\n", __rdpkey_reg());
dprintf1("pkey from siginfo: %jx\n", siginfo_pkey);
*(u64 *)pkey_reg_ptr = 0x;
-   dprintf1("WARNING: set PRKU=0 to allow faulting instruction to 
continue\n");
+   dprintf1("WARNING: set PKEY_REG=0 to allow faulting instruction "
+   "to continue\n");
pkey_faults++;
dprintf1("==\n");
return;
-   if (trapno == 14) {
-   fprintf(stderr,
-   "ERROR: In signal handler, page fault, trapno = %d, ip 
= %016lx\n",
-   trapno, ip);
-   fprintf(stderr, "si_addr %p\n", si->si_addr);
-   fprintf(stderr, "REG_ERR: %lx\n",
-   (unsigned 
long)uctxt->uc_mcontext.gregs[REG_ERR]);
-   exit(1);
-   } else {
-   fprintf(stderr, "unexpected trap %d! at 0x%lx\n", trapno, ip);
-   fprintf(stderr, "si_addr %p\n", si->si_addr);
-   fprintf(stderr, "REG_ERR: %lx\n",
-   (unsigned 
long)uctxt->uc_mcontext.gregs[REG_ERR]);
-   exit(2);
-   }
-   dprint_in_signal = 0;
 }
 
 int wait_all_children(void)
@@ -415,7 +405,7 @@ void pkey_disable_set(int pkey, int flags)
 {
unsigned long syscall_flags = 0;
int ret;
-   int pkey_rights;
+   u32 pkey_rights;
pkey_reg_t orig_pkey_reg = rdpkey_reg();
 
dprintf1("START->%s(%d, 0x%x)\n", __func__,
@@ -453,7 +443,7 @@ void pkey_disable_clear(int pkey, int flags)
 {
unsigned long syscall_flags = 0;
int ret;
-   int pkey_rights = pkey_get(pkey, syscall_flags);
+   u32 pkey_rights = pkey_get(pkey, syscall_flags);
pkey_reg_t orig_pkey_reg = rdpkey_reg();
 
pkey_assert(flags & (PKEY_DISABLE_ACCESS | PKEY_DISABLE_WRITE));
@@ -516,9 +506,10 @@ int sys_mprotect_pkey(void *ptr, size_t size, unsigned 
long orig_prot,
return sret;
 }
 
-int sys_pkey_alloc(unsigned long flags, unsigned long init_val)
+int sys_pkey_alloc(unsigned long flags, u64 init_val)
 {
int ret = syscall(SYS_pkey_alloc, flags, init_val);
+
dprintf1("%s(flags=%lx, init_val=%lx) syscall ret: %d errno: %d\n",
__func__, flags, init_val, ret, errno);
return ret;
@@ -542,7 +533,7 @@ void pkey_set_shadow(u32 key, u64 init_val)
 int alloc_pkey(void)
 {
int ret;
-   unsigned long init_val = 0x0;
+   u64 init_val = 0x0;
 
dprintf1("%s()::%d, pkey_reg: 0x%016lx shadow: %016lx\n", __func__,
__LINE__, __rdpkey_reg(), shadow_pkey_reg);
@@ -692,7 +683,9 @@ void

Re: [PATCH v12 12/22] selftests/vm: generic cleanup

Re: [PATCH v12 12/22] selftests/vm: generic cleanup

[PATCH v12 12/22] selftests/vm: generic cleanup

[PATCH v12 12/22] selftests/vm: generic cleanup

4 matches

Site Navigation

Mail list logo

Footer information