[PATCH] selftests/powerpc: Add a test for PROT_SAO

[Michael Ellerman]

PROT_SAO is a powerpc-specific flag to mmap(), and we rely on arch
specific logic to allow it to be passed to mmap().

Add a small test to ensure mmap() accepts PROT_SAO. We don't have a good
way to test that it actually causes the mapping to be created with the
right flags, so for now we just touch the mapping so it's faulted in. In
future we might be able to do something better.

Signed-off-by: Michael Ellerman 
---
 tools/testing/selftests/powerpc/mm/.gitignore |  1 +
 tools/testing/selftests/powerpc/mm/Makefile   |  4 ++-
 tools/testing/selftests/powerpc/mm/prot_sao.c | 42 +++
 tools/testing/selftests/powerpc/utils.h   |  5 
 4 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/mm/prot_sao.c

diff --git a/tools/testing/selftests/powerpc/mm/.gitignore 
b/tools/testing/selftests/powerpc/mm/.gitignore
index b43ade0ec861..e715a3f2fbf4 100644
--- a/tools/testing/selftests/powerpc/mm/.gitignore
+++ b/tools/testing/selftests/powerpc/mm/.gitignore
@@ -1,3 +1,4 @@
 hugetlb_vs_thp_test
 subpage_prot
 tempfile
+prot_sao
\ No newline at end of file
diff --git a/tools/testing/selftests/powerpc/mm/Makefile 
b/tools/testing/selftests/powerpc/mm/Makefile
index ee179e22308c..3bdb96eae558 100644
--- a/tools/testing/selftests/powerpc/mm/Makefile
+++ b/tools/testing/selftests/powerpc/mm/Makefile
@@ -1,13 +1,15 @@
 noarg:
$(MAKE) -C ../
 
-TEST_PROGS := hugetlb_vs_thp_test subpage_prot
+TEST_PROGS := hugetlb_vs_thp_test subpage_prot prot_sao
 TEST_FILES := tempfile
 
 all: $(TEST_PROGS) $(TEST_FILES)
 
 $(TEST_PROGS): ../harness.c
 
+prot_sao: ../utils.c
+
 include ../../lib.mk
 
 tempfile:
diff --git a/tools/testing/selftests/powerpc/mm/prot_sao.c 
b/tools/testing/selftests/powerpc/mm/prot_sao.c
new file mode 100644
index ..611530d43fa9
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mm/prot_sao.c
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2016, Michael Ellerman, IBM Corp.
+ * Licensed under GPLv2.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "utils.h"
+
+#define SIZE (64 * 1024)
+
+int test_prot_sao(void)
+{
+   char *p;
+
+   /* 2.06 or later should support SAO */
+   SKIP_IF(!have_hwcap(PPC_FEATURE_ARCH_2_06));
+
+   /*
+* Ensure we can ask for PROT_SAO.
+* We can't really verify that it does the right thing, but at least we
+* confirm the kernel will accept it.
+*/
+   p = mmap(NULL, SIZE, PROT_READ | PROT_WRITE | PROT_SAO,
+MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+   FAIL_IF(p == MAP_FAILED);
+
+   /* Write to the mapping, to at least cause a fault */
+   memset(p, 0xaa, SIZE);
+
+   return 0;
+}
+
+int main(void)
+{
+   return test_harness(test_prot_sao, "prot-sao");
+}
diff --git a/tools/testing/selftests/powerpc/utils.h 
b/tools/testing/selftests/powerpc/utils.h
index a985cfaa535e..fbd33e52ef8f 100644
--- a/tools/testing/selftests/powerpc/utils.h
+++ b/tools/testing/selftests/powerpc/utils.h
@@ -27,6 +27,11 @@ int test_harness(int (test_function)(void), char *name);
 extern void *get_auxv_entry(int type);
 int pick_online_cpu(void);
 
+static inline bool have_hwcap(unsigned long ftr)
+{
+   return ((unsigned long)get_auxv_entry(AT_HWCAP) & ftr) == ftr;
+}
+
 static inline bool have_hwcap2(unsigned long ftr2)
 {
return ((unsigned long)get_auxv_entry(AT_HWCAP2) & ftr2) == ftr2;
-- 
2.7.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/crash: Rearrange loop condition to avoid out of bounds array access

[Andrew Donnellan]

On 11/07/16 14:17, Suraj Jitindar Singh wrote:

The array crash_shutdown_handles[] has size CRASH_HANDLER_MAX, thus when
we loop over the elements of the list we check crash_shutdown_handles[i]
&& i < CRASH_HANDLER_MAX. However this means that when we increment i to
CRASH_HANDLER_MAX we will perform an out of bound array access checking
the first condition before exiting on the second condition.

To avoid the out of bounds access, simply reorder the loop conditions.

Fixes Coverity bug #128232

Signed-off-by: Suraj Jitindar Singh 


Fixes: 1d1451655bad ("powerpc: Add array bounds checking to 
crash_shutdown_handlers")

Reviewed-by: Andrew Donnellan 

--
Andrew Donnellan  OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com  IBM Australia Limited

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc/crash: Rearrange loop condition to avoid out of bounds array access

[Suraj Jitindar Singh]

The array crash_shutdown_handles[] has size CRASH_HANDLER_MAX, thus when
we loop over the elements of the list we check crash_shutdown_handles[i]
&& i < CRASH_HANDLER_MAX. However this means that when we increment i to
CRASH_HANDLER_MAX we will perform an out of bound array access checking
the first condition before exiting on the second condition.

To avoid the out of bounds access, simply reorder the loop conditions.

Fixes Coverity bug #128232

Signed-off-by: Suraj Jitindar Singh 
---
 arch/powerpc/kernel/crash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/crash.c b/arch/powerpc/kernel/crash.c
index 888bdf1..47b63de 100644
--- a/arch/powerpc/kernel/crash.c
+++ b/arch/powerpc/kernel/crash.c
@@ -351,7 +351,7 @@ void default_machine_crash_shutdown(struct pt_regs *regs)
old_handler = __debugger_fault_handler;
__debugger_fault_handler = handle_fault;
crash_shutdown_cpu = smp_processor_id();
-   for (i = 0; crash_shutdown_handles[i] && i < CRASH_HANDLER_MAX; i++) {
+   for (i = 0; i < CRASH_HANDLER_MAX && crash_shutdown_handles[i]; i++) {
if (setjmp(crash_shutdown_buf) == 0) {
/*
 * Insert syncs and delay to ensure
-- 
2.5.5

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH V2 2/2] tty/hvc: Use opal irqchip interface if available

[Samuel Mendoza-Jonas]

Update the hvc driver to use the OPAL irqchip if made available by the
running firmware. If it is not present, the driver falls back to the
existing OPAL event number.

Signed-off-by: Samuel Mendoza-Jonas 
Cc:  # 4.1.x-
---
v2: Always try irq_of_parse_and_map before falling back

 drivers/tty/hvc/hvc_opal.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
index b7cd0ae..5107993 100644
--- a/drivers/tty/hvc/hvc_opal.c
+++ b/drivers/tty/hvc/hvc_opal.c
@@ -214,7 +214,13 @@ static int hvc_opal_probe(struct platform_device *dev)
dev->dev.of_node->full_name,
boot ? " (boot console)" : "");
 
-   irq = opal_event_request(ilog2(OPAL_EVENT_CONSOLE_INPUT));
+   irq = irq_of_parse_and_map(dev->dev.of_node, 0);
+   if (!irq) {
+   pr_info("hvc%d: No interrupts property, using OPAL event\n",
+   termno);
+   irq = opal_event_request(ilog2(OPAL_EVENT_CONSOLE_INPUT));
+   }
+
if (!irq) {
pr_err("hvc_opal: Unable to map interrupt for device %s\n",
dev->dev.of_node->full_name);
-- 
2.9.0

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH V2 1/2] tty/hvc: Use IRQF_SHARED for OPAL hvc consoles

[Samuel Mendoza-Jonas]

Commit 2def86a7200c
("hvc: Convert to using interrupts instead of opal events")
enabled the use of interrupts in the hvc_driver for OPAL platforms.
However on machines with more than one hvc console, any console after
the first will fail to register an interrupt handler in
notifier_add_irq() since all consoles share the same IRQ number but do
not set the IRQF_SHARED flag:

[   51.179907] genirq: Flags mismatch irq 31.  (hvc_console) vs.
 (hvc_console)
[   51.180010] hvc_open: request_irq failed with rc -16.

This error propagates up to hvc_open() and the console is closed, but
OPAL will still generate interrupts that are not handled, leading to
rcu_sched stall warnings.

Set IRQF_SHARED when calling request_irq, allowing additional consoles
to start properly. This is only set for consoles handled by
hvc_opal_probe(), leaving other types unaffected.

Signed-off-by: Samuel Mendoza-Jonas 
Cc:  # 4.1.x-
---
 drivers/tty/hvc/hvc_console.h | 1 +
 drivers/tty/hvc/hvc_irq.c | 7 +--
 drivers/tty/hvc/hvc_opal.c| 3 +++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
index 9131019..798c48d 100644
--- a/drivers/tty/hvc/hvc_console.h
+++ b/drivers/tty/hvc/hvc_console.h
@@ -60,6 +60,7 @@ struct hvc_struct {
struct winsize ws;
struct work_struct tty_resize;
struct list_head next;
+   unsigned long flags;
 };
 
 /* implemented by a low level driver */
diff --git a/drivers/tty/hvc/hvc_irq.c b/drivers/tty/hvc/hvc_irq.c
index c9adb05..57d9df7 100644
--- a/drivers/tty/hvc/hvc_irq.c
+++ b/drivers/tty/hvc/hvc_irq.c
@@ -14,6 +14,9 @@ static irqreturn_t hvc_handle_interrupt(int irq, void 
*dev_instance)
/* if hvc_poll request a repoll, then kick the hvcd thread */
if (hvc_poll(dev_instance))
hvc_kick();
+   /* We're safe to always return IRQ_HANDLED as the hvcd thread will
+* iterate through each hvc_struct
+*/
return IRQ_HANDLED;
 }
 
@@ -28,8 +31,8 @@ int notifier_add_irq(struct hvc_struct *hp, int irq)
hp->irq_requested = 0;
return 0;
}
-   rc = request_irq(irq, hvc_handle_interrupt, 0,
-  "hvc_console", hp);
+   rc = request_irq(irq, hvc_handle_interrupt, hp->flags,
+   "hvc_console", hp);
if (!rc)
hp->irq_requested = 1;
return rc;
diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
index 47b54c6..b7cd0ae 100644
--- a/drivers/tty/hvc/hvc_opal.c
+++ b/drivers/tty/hvc/hvc_opal.c
@@ -224,6 +224,9 @@ static int hvc_opal_probe(struct platform_device *dev)
hp = hvc_alloc(termno, irq, ops, MAX_VIO_PUT_CHARS);
if (IS_ERR(hp))
return PTR_ERR(hp);
+
+   /* hvc consoles on powernv may need to share a single irq */
+   hp->flags = IRQF_SHARED;
dev_set_drvdata(>dev, hp);
 
return 0;
-- 
2.9.0

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v4 3/9] kexec_file: Factor out kexec_locate_mem_hole from kexec_add_buffer.

[Dave Young]

On 07/07/16 at 01:23pm, Thiago Jung Bauermann wrote:
> kexec_locate_mem_hole will be used by the PowerPC kexec_file_load
> implementation to find free memory for the purgatory stack.
> 
> Signed-off-by: Thiago Jung Bauermann 
> Cc: Eric Biederman 
> Cc: Dave Young 
> ---
>  include/linux/kexec.h |  1 +
>  kernel/kexec_file.c   | 25 -
>  2 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index ff5fa7707bd7..803f563df81d 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -174,6 +174,7 @@ struct kexec_buf {
>  int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
>  int (*func)(u64, u64, void *));
>  extern int kexec_add_buffer(struct kexec_buf *kbuf);
> +int kexec_locate_mem_hole(struct kexec_buf *kbuf);
>  #endif /* CONFIG_KEXEC_FILE */
>  
>  struct kimage {
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 82ccfc4ee97e..3125d1689712 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -450,6 +450,23 @@ int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
>  }
>  
>  /**
> + * kexec_locate_mem_hole - find free memory for the purgatory or the next 
> kernel
> + * @kbuf:Parameters for the memory search.
> + *
> + * On success, kbuf->mem will have the start address of the memory region 
> found.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int kexec_locate_mem_hole(struct kexec_buf *kbuf)
> +{
> + int ret;
> +
> + ret = arch_kexec_walk_mem(kbuf, locate_mem_hole_callback);
> +
> + return ret == 1 ? 0 : -EADDRNOTAVAIL;
> +}
> +
> +/**
>   * kexec_add_buffer - place a buffer in a kexec segment
>   * @kbuf:Buffer contents and memory parameters.
>   *
> @@ -489,11 +506,9 @@ int kexec_add_buffer(struct kexec_buf *kbuf)
>   kbuf->buf_align = max(kbuf->buf_align, PAGE_SIZE);
>  
>   /* Walk the RAM ranges and allocate a suitable range for the buffer */
> - ret = arch_kexec_walk_mem(kbuf, locate_mem_hole_callback);
> - if (ret != 1) {
> - /* A suitable memory range could not be found for buffer */
> - return -EADDRNOTAVAIL;
> - }
> + ret = kexec_locate_mem_hole(kbuf);
> + if (ret)
> + return ret;
>  
>   /* Found a suitable memory range */
>   ksegment = >image->segment[kbuf->image->nr_segments];
> -- 
> 1.9.1
> 

Acked-by: Dave Young 

Thanks
Dave
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v4 2/9] kexec_file: Change kexec_add_buffer to take kexec_buf as argument.

[Dave Young]

On 07/07/16 at 01:23pm, Thiago Jung Bauermann wrote:
> Adapt all callers to the new function prototype.
> 
> In addition, change the type of kexec_buf.buffer from char * to void *.
> There is no particular reason for it to be a char *, and the change
> allows us to get rid of 3 existing casts to char * in the code.
> 
> Signed-off-by: Thiago Jung Bauermann 
> Cc: Eric Biederman 
> Cc: Dave Young 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: "H. Peter Anvin" 
> Cc: x...@kernel.org
> ---
>  arch/x86/kernel/crash.c   | 37 
>  arch/x86/kernel/kexec-bzimage64.c | 48 +++--
>  include/linux/kexec.h |  8 +---
>  kernel/kexec_file.c   | 88 
> ++-
>  4 files changed, 87 insertions(+), 94 deletions(-)
> 
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 9ef978d69c22..dc026ea361dc 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -615,9 +615,9 @@ static int determine_backup_region(u64 start, u64 end, 
> void *arg)
>  
>  int crash_load_segments(struct kimage *image)
>  {
> - unsigned long src_start, src_sz, elf_sz;
> - void *elf_addr;
>   int ret;
> + struct kexec_buf kbuf = { .image = image, .buf_min = 0,
> +   .buf_max = ULONG_MAX, .top_down = false };
>  
>   /*
>* Determine and load a segment for backup area. First 640K RAM
> @@ -631,43 +631,44 @@ int crash_load_segments(struct kimage *image)
>   if (ret < 0)
>   return ret;
>  
> - src_start = image->arch.backup_src_start;
> - src_sz = image->arch.backup_src_sz;
> -
>   /* Add backup segment. */
> - if (src_sz) {
> + if (image->arch.backup_src_sz) {
> + kbuf.buffer = _zero_bytes;
> + kbuf.bufsz = sizeof(crash_zero_bytes);
> + kbuf.memsz = image->arch.backup_src_sz;
> + kbuf.buf_align = PAGE_SIZE;
>   /*
>* Ideally there is no source for backup segment. This is
>* copied in purgatory after crash. Just add a zero filled
>* segment for now to make sure checksum logic works fine.
>*/
> - ret = kexec_add_buffer(image, (char *)_zero_bytes,
> -sizeof(crash_zero_bytes), src_sz,
> -PAGE_SIZE, 0, -1, 0,
> ->arch.backup_load_addr);
> + ret = kexec_add_buffer();
>   if (ret)
>   return ret;
> + image->arch.backup_load_addr = kbuf.mem;
>   pr_debug("Loaded backup region at 0x%lx backup_start=0x%lx 
> memsz=0x%lx\n",
> -  image->arch.backup_load_addr, src_start, src_sz);
> +  image->arch.backup_load_addr,
> +  image->arch.backup_src_start, kbuf.memsz);
>   }
>  
>   /* Prepare elf headers and add a segment */
> - ret = prepare_elf_headers(image, _addr, _sz);
> + ret = prepare_elf_headers(image, , );
>   if (ret)
>   return ret;
>  
> - image->arch.elf_headers = elf_addr;
> - image->arch.elf_headers_sz = elf_sz;
> + image->arch.elf_headers = kbuf.buffer;
> + image->arch.elf_headers_sz = kbuf.bufsz;
>  
> - ret = kexec_add_buffer(image, (char *)elf_addr, elf_sz, elf_sz,
> - ELF_CORE_HEADER_ALIGN, 0, -1, 0,
> - >arch.elf_load_addr);
> + kbuf.memsz = kbuf.bufsz;
> + kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
> + ret = kexec_add_buffer();
>   if (ret) {
>   vfree((void *)image->arch.elf_headers);
>   return ret;
>   }
> + image->arch.elf_load_addr = kbuf.mem;
>   pr_debug("Loaded ELF headers at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> -  image->arch.elf_load_addr, elf_sz, elf_sz);
> +  image->arch.elf_load_addr, kbuf.bufsz, kbuf.bufsz);
>  
>   return ret;
>  }
> diff --git a/arch/x86/kernel/kexec-bzimage64.c 
> b/arch/x86/kernel/kexec-bzimage64.c
> index f2356bda2b05..4b3a75329fb6 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -331,17 +331,17 @@ static void *bzImage64_load(struct kimage *image, char 
> *kernel,
>  
>   struct setup_header *header;
>   int setup_sects, kern16_size, ret = 0;
> - unsigned long setup_header_size, params_cmdline_sz, params_misc_sz;
> + unsigned long setup_header_size, params_cmdline_sz;
>   struct boot_params *params;
>   unsigned long bootparam_load_addr, kernel_load_addr, initrd_load_addr;
>   unsigned long purgatory_load_addr;
> - unsigned long kernel_bufsz, kernel_memsz, kernel_align;
> - char *kernel_buf;
>   struct bzimage64_data *ldata;
>   struct kexec_entry64_regs 

Re: [PATCH v4 1/9] kexec_file: Allow arch-specific memory walking for kexec_add_buffer

[Dave Young]

On 07/07/16 at 01:23pm, Thiago Jung Bauermann wrote:
> Allow architectures to specify a different memory walking function for
> kexec_add_buffer. x86 uses iomem to track reserved memory ranges, but
> PowerPC uses the memblock subsystem.
> 
> Signed-off-by: Thiago Jung Bauermann 
> Cc: Eric Biederman 
> Cc: Dave Young 
> ---
>  include/linux/kexec.h   | 29 -
>  kernel/kexec_file.c | 30 ++
>  kernel/kexec_internal.h | 16 
>  3 files changed, 50 insertions(+), 25 deletions(-)
> 
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index e8acb2b43dd9..2549a99a748c 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -146,7 +146,34 @@ struct kexec_file_ops {
>   kexec_verify_sig_t *verify_sig;
>  #endif
>  };
> -#endif
> +
> +/**
> + * struct kexec_buf - parameters for finding a place for a buffer in memory
> + * @image:   kexec image in which memory to search.
> + * @buffer:  Contents which will be copied to the allocated memory.
> + * @bufsz:   Size of @buffer.
> + * @mem: On return will have address of the buffer in memory.
> + * @memsz:   Size for the buffer in memory.
> + * @buf_align:   Minimum alignment needed.
> + * @buf_min: The buffer can't be placed below this address.
> + * @buf_max: The buffer can't be placed above this address.
> + * @top_down:Allocate from top of memory.
> + */
> +struct kexec_buf {
> + struct kimage *image;
> + char *buffer;
> + unsigned long bufsz;
> + unsigned long mem;
> + unsigned long memsz;
> + unsigned long buf_align;
> + unsigned long buf_min;
> + unsigned long buf_max;
> + bool top_down;
> +};
> +
> +int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
> +int (*func)(u64, u64, void *));
> +#endif /* CONFIG_KEXEC_FILE */
>  
>  struct kimage {
>   kimage_entry_t head;
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 01ab82a40d22..9c0c565a08db 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -428,6 +428,27 @@ static int locate_mem_hole_callback(u64 start, u64 end, 
> void *arg)
>   return locate_mem_hole_bottom_up(start, end, kbuf);
>  }
>  
> +/**
> + * arch_kexec_walk_mem - call func(data) on free memory regions
> + * @kbuf:Context info for the search. Also passed to @func.
> + * @func:Function to call for each memory region.
> + *
> + * Return: The memory walk will stop when func returns a non-zero value
> + * and that value will be returned. If all free regions are visited without
> + * func returning non-zero, then zero will be returned.
> + */
> +int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
> +int (*func)(u64, u64, void *))
> +{
> + if (kbuf->image->type == KEXEC_TYPE_CRASH)
> + return walk_iomem_res_desc(crashk_res.desc,
> +IORESOURCE_SYSTEM_RAM | 
> IORESOURCE_BUSY,
> +crashk_res.start, crashk_res.end,
> +kbuf, func);
> + else
> + return walk_system_ram_res(0, ULONG_MAX, kbuf, func);
> +}
> +
>  /*
>   * Helper function for placing a buffer in a kexec segment. This assumes
>   * that kexec_mutex is held.
> @@ -474,14 +495,7 @@ int kexec_add_buffer(struct kimage *image, char *buffer, 
> unsigned long bufsz,
>   kbuf->top_down = top_down;
>  
>   /* Walk the RAM ranges and allocate a suitable range for the buffer */
> - if (image->type == KEXEC_TYPE_CRASH)
> - ret = walk_iomem_res_desc(crashk_res.desc,
> - IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
> - crashk_res.start, crashk_res.end, kbuf,
> - locate_mem_hole_callback);
> - else
> - ret = walk_system_ram_res(0, -1, kbuf,
> -   locate_mem_hole_callback);
> + ret = arch_kexec_walk_mem(kbuf, locate_mem_hole_callback);
>   if (ret != 1) {
>   /* A suitable memory range could not be found for buffer */
>   return -EADDRNOTAVAIL;
> diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
> index 0a52315d9c62..4cef7e4706b0 100644
> --- a/kernel/kexec_internal.h
> +++ b/kernel/kexec_internal.h
> @@ -20,22 +20,6 @@ struct kexec_sha_region {
>   unsigned long len;
>  };
>  
> -/*
> - * Keeps track of buffer parameters as provided by caller for requesting
> - * memory placement of buffer.
> - */
> -struct kexec_buf {
> - struct kimage *image;
> - char *buffer;
> - unsigned long bufsz;
> - unsigned long mem;
> - unsigned long memsz;
> - unsigned long buf_align;
> - unsigned long buf_min;
> - unsigned long buf_max;
> - bool top_down;  /* allocate from top of memory hole */
> -};
> -
>  void 

Re: [RFC] arm64: kexec_file_load support

[Dave Young]

On 07/08/16 at 11:48am, Thiago Jung Bauermann wrote:
> Am Donnerstag, 07 Juli 2016, 14:12:45 schrieb Dave Young:
> > If so maybe change a bit from your precious mentioned 7 args proposal like
> > below?
> > 
> > struct kexec_file_fd {
> > enum kexec_file_type;
> > int fd;
> > }
> > 
> > struct kexec_fdset {
> > int nr_fd;
> > struct kexec_file_fd fd[0];
> > }
> > 
> > int kexec_file_load(int kernel_fd, int initrd_fd,
> > unsigned long cmdline_len, const char *cmdline_ptr,
> > unsigned long flags, struct kexec_fdset *extra_fds);
> 
> 
> Is there a way for the kernel to distinguish whether the process passed 5 or 
> 6 arguments? How can it know whether extra_fds is a valid argument or just 
> garbage? I think we have to define a new flag KEXEC_FILE_EXTRA_FDS so that 
> the process can signal that it is using the new interface.

Agreed, a new flag is needed.

Thanks
Dave
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v4 3/9] kexec_file: Factor out kexec_locate_mem_hole from kexec_add_buffer.

[Dave Young]

On 07/10/16 at 04:11pm, Michael Ellerman wrote:
> Thiago Jung Bauermann  writes:
> 
> > kexec_locate_mem_hole will be used by the PowerPC kexec_file_load
> > implementation to find free memory for the purgatory stack.
> >
> > Signed-off-by: Thiago Jung Bauermann 
> > Cc: Eric Biederman 
> > Cc: Dave Young 
> 
> Dave are you happy with the first three patches? If so do you mind
> sending an ack?

I reviewed the 3 patches, they look good to me. Will ack after a small test 
today.

> 
> Given the series touches generic code, x86 and powerpc this might be one
> for Andrew to take. Or is there a kexec tree it should go through?

They should go Andrew's tree, but I'm not sure about the powerpc part.

Thanks
Dave
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] include: mman: Use bool instead of int for the return value of arch_validate_prot

[Dave Hansen]

On 07/09/2016 09:29 AM, cheng...@emindsoft.com.cn wrote:
> -static inline int arch_validate_prot(unsigned long prot)
> +static inline bool arch_validate_prot(unsigned long prot)
>  {
>   if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
> - return 0;
> - if ((prot & PROT_SAO) && !cpu_has_feature(CPU_FTR_SAO))
> - return 0;
> - return 1;
> + return false;
> + return (prot & PROT_SAO) == 0 || cpu_has_feature(CPU_FTR_SAO);
>  }
>  #define arch_validate_prot(prot) arch_validate_prot(prot)

Please don't do things like this.  They're not obviously correct and
also have no obvious benefit.  You also don't mention why you bothered
to alter the logical structure of these checks.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Linux 4.7: Reported regressions as of Sunday, 2016-07-10

[Thorsten Leemhuis]

Hi! Here is my fifth regression report for Linux 4.7. It lists 10
regressions I'm currently aware of; 2 of them are new; 1 of those 
seems to be a a side effect of a fix for another regression.

The report also mentions 3 regression that I removed from the list, as
it looks like those issues are no regressions.

Find also details on 4 regressions that were fixed since the last
report(¹)

As usual: Please let me know about any regression missing on the list  
or if it contains something which shouldn't be there.

HTH, CU, Thorsten

(¹) previous reports can be found at
http://thread.gmane.org/gmane.linux.kernel/2241805
http://thread.gmane.org/gmane.linux.kernel/2247804
http://thread.gmane.org/gmane.linux.kernel/2253623
http://thread.gmane.org/gmane.linux.kernel/2258287

== Current regressions ==

Desc: Bad flicker on skylake HQD due to code in the 4.7 merge window
Repo: 2016-05-30 http://thread.gmane.org/gmane.linux.kernel/2230377
Stat: 2016-07-08 http://thread.gmane.org/gmane.linux.kernel/2230377/focus=94140
Note: vswing issue, maybe primary a Firmware problem; a few patches improved 
situation, but did not fix the problem

Desc: 795ae7a0de: pixz.throughput -9.1% regression
Repo: 2016-06-02 http://thread.gmane.org/gmane.linux.kernel/2233056/
Stat: 2016-06-22 
http://thread.gmane.org/gmane.linux.kernel/2233056/focus=2251134
Note: hannes still can't reproduce; stalled until reporter posts results from 
more tests

Desc: System hang when plug/un-plug USB 3.1 key via thunderbolt port on Dell 
XPS 13
Repo: 2016-06-14 https://bugzilla.kernel.org/show_bug.cgi?id=120241
Stat: 2016-07-08 https://bugzilla.kernel.org/show_bug.cgi?id=120241#c7
Note: there was wip, but root cause hard to track down; responsible developer 
now afk till early August

Desc: regression in 8250 uart driver
Repo: 2016-06-14 
http://thread.gmane.org/gmane.linux.kernel/2243130/focus=2243653
Stat: 2016-07-02 
http://thread.gmane.org/gmane.linux.kernel/2243130/focus=2258232
Note: andy has it still on his todo list, but something else has a higher 
priority

Desc: Multi-thread udp 4.7 regression, bisected to 71d8c47fc653; performance 
decreased or complete failure with a test
Repo: 2016-06-27 http://thread.gmane.org/gmane.linux.network/418861/focus=418908
Stat: 2016-07-05 http://thread.gmane.org/gmane.linux.network/418861/focus=63936
Note: waiting for feedback from reporter

Desc: kmemcheck: Caught 64-bit read from uninitialized memory; 
iptables/nf_register_net_hooks
Repo: 2016-06-19 https://bugzilla.kernel.org/show_bug.cgi?id=120651
Stat: 2016-07-02 
Note: afaics stalled, after I told reporter a week ago how to contact the 
relevant developers

Desc: UBSAN splat in drivers/acpi/acpica/dsutils.c:641:16
Repo: 2016-06-15 https://bugzilla.kernel.org/show_bug.cgi?id=120351
Stat: 2016-06-22 https://bugzilla.kernel.org/show_bug.cgi?id=120411#c3
Note: wonder if this is important enough to investigate further; related, 
similar bugs: https://bugzilla.kernel.org/show_bug.cgi?id=120411 
https://bugzilla.kernel.org/show_bug.cgi?id=120391 
https://bugzilla.kernel.org/show_bug.cgi?id=120371 
https://bugzilla.kernel.org/show_bug.cgi?id=120361 Not sure which of them are 
regressions

Desc: Kernel does not boot with 7ed18e2d1b6782989eb399ef79a8cc1a1b583b3c ( 
acpi-4.7-rc7 aka All of these fix recent regressions in ACPICA, in the ACPI PCI 
IRQ management code and in the ACPI AML debugger.)
Repo: 2016-07-08 https://bugzilla.kernel.org/show_bug.cgi?id=121701
Stat: 2016-07-10 
http://thread.gmane.org/gmane.linux.kernel.pci/53279/focus=85652
Note: pointed Rafael to bugzilla on LKML

Desc: ACPI EC problems/ ACPI / EC: Fix an order issue in ec_remove_handlers()
Repo: 2016-07-06 http://thread.gmane.org/gmane.linux.kernel/2260279/
Stat: 2016-07-08 http://thread.gmane.org/gmane.linux.kernel/2260279/focus=85583
Note: patch under discussion


== Going to me removed from the list ==

Desc: RadeonSI get a huge performance dip with used with the nine state tracker
Repo: 2016-06-04 https://bugzilla.kernel.org/show_bug.cgi?id=119631
Stat: 2016-07-04 https://bugzilla.kernel.org/show_bug.cgi?id=119631#c14
Note: not a regression according to one of the AMD graphic driver developers; 
otoh it looks like one to me; yes, the real bug is in the gallium nine, wine 
side, but afaics it only showed up after a kernel change introduced a new 
feature

Desc: performance drop on SFC interface around 30 %
Repo: 2016-06-17 https://bugzilla.kernel.org/show_bug.cgi?id=120461
Stat: 2016-07-06 https://bugzilla.kernel.org/show_bug.cgi?id=120461#c20
Note: reporter and developer agree: not a regression in 4.7

Desc: Rename file corruption due to VFS-Changes?
Repo: 2016-06-16 http://thread.gmane.org/gmane.linux.kernel/2245402/
Stat: 2016-06-22 
http://thread.gmane.org/gmane.linux.kernel/2245402/focus=2250254
Note: issue likely fixed in mainline by 9f541801 + e7d6ef979; see also: 
http://thread.gmane.org/gmane.linux.kernel/2231928/focus=76124


== Fixed since last report ==

Desc: BUG: unable to handle 

Re: [PATCH 0/9] mm: Hardened usercopy

[Andy Lutomirski]

On Sun, Jul 10, 2016 at 5:03 AM, PaX Team  wrote:
> On 10 Jul 2016 at 11:16, Ingo Molnar wrote:
>
>> * PaX Team  wrote:
>>
>> > On 9 Jul 2016 at 14:27, Andy Lutomirski wrote:
>> >
>> > > I like the series, but I have one minor nit to pick.  The effect of this
>> > > series is to harden usercopy, but most of the code is really about
>> > > infrastructure to validate that a pointed-to object is valid.
>> >
>> > actually USERCOPY has never been about validating pointers. its sole 
>> > purpose is
>> > to validate the *size* argument of copy*user calls, a very specific form of
>> > runtime bounds checking.
>>
>> What this code has been about originally is largely immaterial, unless you 
>> can
>> formulate it into a technical argument.
>
> we design defense mechanisms for specific and clear purposes, starting with
> a threat model, evaluating defense options based on various criteria, etc.
> USERCOPY underwent this same process and taking it out of its original context
> means that all you get in the end is cargo cult security (wouldn't be the 
> first
> time it has happened (ExecShield, ASLR, etc)).
>
> that said, i actually started that discussion but for some reason you chose
> not to respond to that one part of my mail so let me ask it again:
>
>   what kind of checks are you thinking of here? and more fundamentally, 
> against
>   what kind of threats?
>
> as far as i'm concerned, a defense mechanism is only as good as its underlying
> threat model. by validating pointers (for yet to be stated security related
> properties) you're presumably assuming some kind of threat and unless stated
> clearly what that threat is (unintended pointer modification through memory
> corruption and/or other bugs?) noone can tell whether the proposed defense
> mechanism will actually be effective in preventing exploitation. it is the
> worst kind of defense that doesn't actually achieve its stated goals, that
> way lies false sense of security and i hope noone here is in that business.

I'm imaging security bugs that involve buffer length corruption but
that don't call copy_to/from_user.  Hardened usercopy shuts
expoitation down if the first use of the corrupt size is
copy_to/from_user or similar.  I bet that a bit better coverage could
be achieved by instrumenting more functions.

To be clear: I'm not objecting to calling the overall feature hardened
usercopy or similar.  I object to
CONFIG_HAVE_HARDENED_USERCOPY_ALLOCATOR.  That feature is *used* for
hardened usercopy but is not, in and of itself, a usercopy thing.
It's an object / memory range validation thing.  So we'll feel silly
down the road if we use it for something else and the config option
name has nothing to do with the feature.

>> > [...] like the renaming of .data..read_only to .data..ro_after_init which 
>> > also
>> > had nothing to do with init but everything to do with objects being 
>> > conceptually
>> > read-only...
>>
>> .data..ro_after_init objects get written to during bootup so it's 
>> conceptually
>> quite confusing to name it "read-only" without any clear qualifiers.
>>
>> That it's named consistently with its role of "read-write before init and 
>> read
>> only after init" on the other hand is not confusing at all. Not sure what 
>> your
>> problem is with the new name.
>
> the new name reflects a complete misunderstanding of the PaX feature it was 
> based
> on (typical case of cargo cult security). in particular, the __read_only 
> facility
> in PaX is part of a defense mechanism that attempts to solve a specific 
> problem
> (like everything else) and that problem has nothing whatsoever to do with what
> happens before/after the kernel init process. enforcing read-ony kernel 
> memory at
> the end of kernel initialization is an implementation detail only and wasn't 
> even
> true always (and still isn't true for kernel modules for example): in the 
> linux 2.4
> days PaX actually enforced read-only kernel memory properties in startup_32 
> already
> but i relaxed that for the 2.6+ port as the maintenance cost (finding out and
> handling new exceptional cases) wasn't worth it.
>
> also naming things after their implementation is poor taste and can result in
> even bigger problems down the line since as soon as the implementation 
> changes,
> you will have a flag day or have to keep a bad name. this is a lesson that the
> REFCOUNT submission will learn too since the kernel's atomic*_t types (an
> implementation detail) are used extensively for different purposes, instead of
> using specialized types (kref is a good example of that). for 
> .data..ro_after_init
> the lesson will happen when you try to add back the remaining pieces from PaX,
> such as module handling and not-always-const-in-the-C-sense objects and 
> associated
> accessors.

The name is related to how the thing works.  If I understand
correctly, in PaX, the idea is to make some things readonly and use
pax_open_kernel(), etc to write 

Re: [PATCH 0/9] mm: Hardened usercopy

[PaX Team]

On 10 Jul 2016 at 11:16, Ingo Molnar wrote:

> * PaX Team  wrote:
> 
> > On 9 Jul 2016 at 14:27, Andy Lutomirski wrote:
> > 
> > > I like the series, but I have one minor nit to pick.  The effect of this 
> > > series is to harden usercopy, but most of the code is really about 
> > > infrastructure to validate that a pointed-to object is valid.
> > 
> > actually USERCOPY has never been about validating pointers. its sole 
> > purpose is 
> > to validate the *size* argument of copy*user calls, a very specific form of 
> > runtime bounds checking.
> 
> What this code has been about originally is largely immaterial, unless you 
> can 
> formulate it into a technical argument.

we design defense mechanisms for specific and clear purposes, starting with
a threat model, evaluating defense options based on various criteria, etc.
USERCOPY underwent this same process and taking it out of its original context
means that all you get in the end is cargo cult security (wouldn't be the first
time it has happened (ExecShield, ASLR, etc)).

that said, i actually started that discussion but for some reason you chose
not to respond to that one part of my mail so let me ask it again:

  what kind of checks are you thinking of here? and more fundamentally, against
  what kind of threats?

as far as i'm concerned, a defense mechanism is only as good as its underlying
threat model. by validating pointers (for yet to be stated security related
properties) you're presumably assuming some kind of threat and unless stated
clearly what that threat is (unintended pointer modification through memory
corruption and/or other bugs?) noone can tell whether the proposed defense
mechanism will actually be effective in preventing exploitation. it is the
worst kind of defense that doesn't actually achieve its stated goals, that
way lies false sense of security and i hope noone here is in that business.

i note that this analysis is also missing from this USERCOPY submission except
for stating what Kees assumed about USERCOPY (and apparently noone could be
bothered to read the original Kconfig help of it which clearly states that the
purpose is copy size checking, not some elaborate pointer validation, the latter
is an implementation detail only and is necessary to be able to derive the
underlying slab object's intended size).

> There are a number of cheap tests we can do and there are a number of ways 
> how a 
> 'pointer' can be validated runtime, without any 'size' information:
> 
>  - for example if a pointer points into a red zone straight away then we know 
> it's
>bogus.

it's not pointer validation but bounds checking: you already know which memory
object the pointer is supposed to point to, you only check its bounds. if it was
an attacker controlled pointer then all this would be a pointless check of 
course,
trivial for an attacker to circumvent (and this is why it's not part of the
USERCOPY design).

>  - or if a kernel pointer is points outside the valid kernel virtual memory 
> range
>we know it's bogus as well.

accesses outside of valid virtual memory will cause a page fault ('oops' in 
linux
terms), there's no need to explicitly check for that.

> So while only doing a bounds check might have been the original purpose of 
> the 
> patch set, Andy's point is that it might make sense to treat this facility as 
> a 
> more generic 'object validation' code of (pointer,size) object and not limit 
> it to 
> 'runtime bounds checking'.

FYI, 'runtime bounds checking' is a terminus technicus and it is about 
validating
both the pointer and underlying object's size. that's the reason i called 
USERCOPY
a 'very specific form' of it only since it doesn't validate each part equally 
well
(or well enough at all, even the size check is not as precise as it could be).

as for what does or doesn't make sense, first you'll have to define a threat
model and evaluate everything else based on that. since noone has solved the
general bounds checking problem with acceptable properties (mostly performance
impact, but also memory overhead, etc), i'm all ears to hear what you guys have
come up with.

> That kind of extended purpose behind a facility should be reflected in the 
> naming.
> Confusing names are often the source of misunderstandings and bugs.

definitely, but before you bikeshed on naming, you should figure out what and 
why
you want to do, whether it's even feasible, meaningful, useful, etc. answering 
the
opening question and digging into the details is the first step of any design
process, not its naming.

> The 9-patch series as submitted here is neither just 'bounds checking' nor 
> just 
> pure 'pointer checking', it's about validating that a (pointer,size) range of 
> memory passed to a (user) memory copy function is fully within a valid object 
> the 
> kernel might know about (in an fast to check fashion).
> 
> This necessary means:
> 
>  - the start of the range points to a valid object to begin 

Re: [PATCH 0/9] mm: Hardened usercopy

[Ingo Molnar]

* PaX Team  wrote:

> On 9 Jul 2016 at 14:27, Andy Lutomirski wrote:
> 
> > I like the series, but I have one minor nit to pick.  The effect of this 
> > series is to harden usercopy, but most of the code is really about 
> > infrastructure to validate that a pointed-to object is valid.
> 
> actually USERCOPY has never been about validating pointers. its sole purpose 
> is 
> to validate the *size* argument of copy*user calls, a very specific form of 
> runtime bounds checking.

What this code has been about originally is largely immaterial, unless you can 
formulate it into a technical argument.

There are a number of cheap tests we can do and there are a number of ways how 
a 
'pointer' can be validated runtime, without any 'size' information:

 - for example if a pointer points into a red zone straight away then we know 
it's
   bogus.

 - or if a kernel pointer is points outside the valid kernel virtual memory 
range
   we know it's bogus as well.

So while only doing a bounds check might have been the original purpose of the 
patch set, Andy's point is that it might make sense to treat this facility as a 
more generic 'object validation' code of (pointer,size) object and not limit it 
to 
'runtime bounds checking'. That kind of extended purpose behind a facility 
should 
be reflected in the naming.

Confusing names are often the source of misunderstandings and bugs.

The 9-patch series as submitted here is neither just 'bounds checking' nor just 
pure 'pointer checking', it's about validating that a (pointer,size) range of 
memory passed to a (user) memory copy function is fully within a valid object 
the 
kernel might know about (in an fast to check fashion).

This necessary means:

 - the start of the range points to a valid object to begin with (if known)

 - the range itself does not point beyond the end of the object (if known)

 - even if the kernel does not know anything about the pointed to object it can 
   do a pointer check (for example is it pointing inside kernel virtual memory) 
   and do a bounds check on the size.

Do you disagree with that?

> > Might it make sense to call the infrastructure part something else?
> 
> yes, more bikeshedding will surely help, [...]

Insulting and ridiculing a reviewer who explicitly qualified his comments with 
"one minor nit to pick" sure does not help upstream integration either. (Unless 
the goal is to prevent upstream integration.)

> [...] like the renaming of .data..read_only to .data..ro_after_init which 
> also 
> had nothing to do with init but everything to do with objects being 
> conceptually 
> read-only...

.data..ro_after_init objects get written to during bootup so it's conceptually 
quite confusing to name it "read-only" without any clear qualifiers.

That it's named consistently with its role of "read-write before init and read 
only after init" on the other hand is not confusing at all. Not sure what your 
problem is with the new name.

Names within submitted patches get renamed on a routine basis during review. 
It's 
often only minor improvements in naming (which you can consider bike shedding), 
but in this particular case the rename was clearly useful in not just improving 
the name but in avoiding an actively confusing name. So I disagree not just 
with 
the hostile tone of your reply but with your underlying technical point as well.

Thanks,

Ingo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v8 10/11] cpuidle/powernv: Add support for POWER ISA v3 idle states

[Michael Ellerman]

"Shreyas B. Prabhu"  writes:

> POWER ISA v3 defines a new idle processor core mechanism. In summary,
>  a) new instruction named stop is added.
>  b) new per thread SPR named PSSCR is added which controls the behavior
>   of stop instruction.
>
> Supported idle states and value to be written to PSSCR register to enter
> any idle state is exposed via ibm,cpu-idle-state-names and
> ibm,cpu-idle-state-psscr respectively. To enter an idle state,
> platform provided power_stop() needs to be invoked with the appropriate
> PSSCR value.
>
> This patch adds support for this new mechanism in cpuidle powernv driver.
>
> Cc: Rafael J. Wysocki 
> Cc: Daniel Lezcano 
> Cc: Rob Herring 
> Cc: Lorenzo Pieralisi 
> Cc: linux...@vger.kernel.org

Rafael/Daniel do you guys want to ack or nack this?

I'll take silence as "not bothered" and merge the whole series via the
powerpc tree.

cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v4 3/9] kexec_file: Factor out kexec_locate_mem_hole from kexec_add_buffer.

[Michael Ellerman]

Thiago Jung Bauermann  writes:

> kexec_locate_mem_hole will be used by the PowerPC kexec_file_load
> implementation to find free memory for the purgatory stack.
>
> Signed-off-by: Thiago Jung Bauermann 
> Cc: Eric Biederman 
> Cc: Dave Young 

Dave are you happy with the first three patches? If so do you mind
sending an ack?

Given the series touches generic code, x86 and powerpc this might be one
for Andrew to take. Or is there a kexec tree it should go through?

cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev