Re: [PATCH] powerpc: Replace GPL boilerplate with SPDX identifiers
On Wed, 2019-08-28 at 08:07 +0200, Thomas Huth wrote: > The FSF does not reside in "675 Mass Ave, Cambridge" anymore... > let's simply use proper SPDX identifiers instead. > > Signed-off-by: Thomas Huth Acked-by: Russell Currey
Re: [PATCH] powerpc/eeh_cache: fix a W=1 kernel-doc warning
On Wed, 2019-06-05 at 16:46 -0400, Qian Cai wrote: > The opening comment mark "/**" is reserved for kernel-doc comments, > so > it will generate a warning with "make W=1". > > arch/powerpc/kernel/eeh_cache.c:37: warning: cannot understand > function > prototype: 'struct pci_io_addr_range > > Since this is not a kernel-doc for the struct below, but rather an > overview of this source eeh_cache.c, just use the free-form comments > kernel-doc syntax instead. > > Signed-off-by: Qian Cai Looks good to me. Acked-by: Russell Currey
Re: [RFC PATCH] powerpc/mm: Implement STRICT_MODULE_RWX
On Wed, 2019-05-15 at 06:20 +, Christophe Leroy wrote: Confirming this works on hash and radix book3s64. > + > + // only operate on VM areas for now > + area = find_vm_area((void *)addr); > + if (!area || end > (unsigned long)area->addr + area->size || > + !(area->flags & VM_ALLOC)) > + return -EINVAL; https://lore.kernel.org/patchwork/project/lkml/list/?series=391470 With this patch, the above series causes crashes on (at least) Hash, since it adds another user of change_page_rw() and change_page_nx() that for reasons I don't understand yet, we can't handle. I can work around this with: if (area->flags & VM_FLUSH_RESET_PERMS) return 0; so this is broken on at least one platform as of 5.2-rc1. We're going to look into this more to see if there's anything else we have to do as a result of this series before the next merge window, or if just working around it like this is good enough. - Russell
Re: [RFC PATCH] powerpc/mm: Implement STRICT_MODULE_RWX
On Wed, 2019-05-15 at 06:20 +, Christophe Leroy wrote: > Strict module RWX is just like strict kernel RWX, but for modules - > so > loadable modules aren't marked both writable and executable at the > same > time. This is handled by the generic code in kernel/module.c, and > simply requires the architecture to implement the set_memory() set of > functions, declared with ARCH_HAS_SET_MEMORY. > > There's nothing other than these functions required to turn > ARCH_HAS_STRICT_MODULE_RWX on, so turn that on too. > > With STRICT_MODULE_RWX enabled, there are as many W+X pages at > runtime > as there are with CONFIG_MODULES=n (none), so in Russel's testing it > works > well on both Hash and Radix book3s64. > > There's a TODO in the code for also applying the page permission > changes > to the backing pages in the linear mapping: this is pretty simple for > Radix and (seemingly) a lot harder for Hash, so I've left it for now > since there's still a notable security benefit for the patch as-is. > > Technically can be enabled without STRICT_KERNEL_RWX, but > that doesn't gets you a whole lot, so we should leave it off by > default > until we can get STRICT_KERNEL_RWX to the point where it's enabled by > default. > > Signed-off-by: Russell Currey > Signed-off-by: Christophe Leroy > --- Thanks for this, I figured you'd know how to make this work on 32bit too. I'll test on my end today. Note that there are two Ls in my name! To quote the great Rusty, "This Russel disease must be stamped out before it becomes widespread".
Re: [PATCH v2 2/2] powerpc: use probe_user_read()
On Tue, 2019-01-08 at 10:37 +0100, Christophe Leroy wrote:
> Hi Michael and Russell,
>
> Any idea why:
> - checkpatch reports missing Signed-off-by:
> - Snowpatch build fails on PPC64 (it seems unrelated to the patch,
> something wrong in lib/genalloc.c)
Upstream kernel broke for powerpc (snowpatch applies patches on top of
powerpc/next), it was fixed in commit
35004f2e55807a1a1491db24ab512dd2f770a130 which I believe is in
powerpc/next now. I will look at rerunning tests for all the patches
that this impacted.
As for the S-o-b, no clue, I'll have a look. Thanks for the report!
- Russell
>
> Thanks
> Christophe
>
> Le 08/01/2019 à 08:37, Christophe Leroy a écrit :
> > Instead of opencoding, use probe_user_read() to failessly
> > read a user location.
> >
> > Signed-off-by: Christophe Leroy
> > ---
> > v2: Using probe_user_read() instead of probe_user_address()
> >
> > arch/powerpc/kernel/process.c | 12 +---
> > arch/powerpc/mm/fault.c | 6 +-
> > arch/powerpc/perf/callchain.c | 20 +++-
> > arch/powerpc/perf/core-book3s.c | 8 +---
> > arch/powerpc/sysdev/fsl_pci.c | 10 --
> > 5 files changed, 10 insertions(+), 46 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/process.c
> > b/arch/powerpc/kernel/process.c
> > index ce393df243aa..6a4b59d574c2 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -1298,16 +1298,6 @@ void show_user_instructions(struct pt_regs
> > *regs)
> >
> > pc = regs->nip - (NR_INSN_TO_PRINT * 3 / 4 * sizeof(int));
> >
> > - /*
> > -* Make sure the NIP points at userspace, not kernel text/data
> > or
> > -* elsewhere.
> > -*/
> > - if (!__access_ok(pc, NR_INSN_TO_PRINT * sizeof(int), USER_DS))
> > {
> > - pr_info("%s[%d]: Bad NIP, not dumping instructions.\n",
> > - current->comm, current->pid);
> > - return;
> > - }
> > -
> > seq_buf_init(, buf, sizeof(buf));
> >
> > while (n) {
> > @@ -1318,7 +1308,7 @@ void show_user_instructions(struct pt_regs
> > *regs)
> > for (i = 0; i < 8 && n; i++, n--, pc += sizeof(int)) {
> > int instr;
> >
> > - if (probe_kernel_address((const void *)pc,
> > instr)) {
> > + if (probe_user_read(, (void __user *)pc,
> > sizeof(instr))) {
> > seq_buf_printf(, " ");
> > continue;
> > }
> > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> > index 887f11bcf330..ec74305fa330 100644
> > --- a/arch/powerpc/mm/fault.c
> > +++ b/arch/powerpc/mm/fault.c
> > @@ -276,12 +276,8 @@ static bool bad_stack_expansion(struct pt_regs
> > *regs, unsigned long address,
> > if ((flags & FAULT_FLAG_WRITE) && (flags &
> > FAULT_FLAG_USER) &&
> > access_ok(nip, sizeof(*nip))) {
> > unsigned int inst;
> > - int res;
> >
> > - pagefault_disable();
> > - res = __get_user_inatomic(inst, nip);
> > - pagefault_enable();
> > - if (!res)
> > + if (!probe_user_read(, nip, sizeof(inst)))
> > return !store_updates_sp(inst);
> > *must_retry = true;
> > }
> > diff --git a/arch/powerpc/perf/callchain.c
> > b/arch/powerpc/perf/callchain.c
> > index 0af051a1974e..0680efb2237b 100644
> > --- a/arch/powerpc/perf/callchain.c
> > +++ b/arch/powerpc/perf/callchain.c
> > @@ -159,12 +159,8 @@ static int read_user_stack_64(unsigned long
> > __user *ptr, unsigned long *ret)
> > ((unsigned long)ptr & 7))
> > return -EFAULT;
> >
> > - pagefault_disable();
> > - if (!__get_user_inatomic(*ret, ptr)) {
> > - pagefault_enable();
> > + if (!probe_user_read(ret, ptr, sizeof(*ret)))
> > return 0;
> > - }
> > - pagefault_enable();
> >
> > return read_user_stack_slow(ptr, ret, 8);
> > }
> > @@ -175,12 +171,8 @@ static int read_user_stack_32(unsigned int
> > __user *ptr, unsigned int *ret)
> > ((unsigned long)ptr & 3))
> > return -EFAULT;
> >
> > - pagefault_disable();
> > - if (!__get_user_inatomic(*ret, ptr)) {
> > - pagefault_enable();
> > + if (!probe_user_read(ret, ptr, sizeof(*ret)))
> > return 0;
> > - }
> > - pagefault_enable();
> >
> > return read_user_stack_slow(ptr, ret, 4);
> > }
> > @@ -307,17 +299,11 @@ static inline int current_is_64bit(void)
> >*/
> > static int read_user_stack_32(unsigned int __user *ptr, unsigned
> > int *ret)
> > {
> > - int rc;
> > -
> > if ((unsigned long)ptr > TASK_SIZE - sizeof(unsigned int) ||
> > ((unsigned long)ptr & 3))
> > return -EFAULT;
> >
> > - pagefault_disable();
> > - rc = __get_user_inatomic(*ret, ptr);
> > -
Re: [PATCH -next] powerpc/eeh: Fix debugfs_simple_attr.cocci warnings
On Thu, 2018-12-20 at 02:42 +, YueHaibing wrote: > Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE > for debugfs files. > > Semantic patch information: > Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file() > imposes some significant overhead as compared to > DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe(). > > Generated by: > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci > > Signed-off-by: YueHaibing Acked-by: Russell Currey
Re: [RFC PATCH v2 11/11] powerpc/book3s32: Implement Kernel Userspace Access Protection
On Wed, 2018-11-28 at 09:27 +, Christophe Leroy wrote: > This patch implements Kernel Userspace Access Protection for > book3s/32. > > Due to limitations of the processor page protection capabilities, > the protection is only against writing. read protection cannot be > achieved using page protection. > > In order to provide the protection, Ku and Ks keys are modified in > Userspace Segment registers, and different PP bits are used to: > > PP01 provides RW for Key 0 and RO for Key 1 > PP10 provides RW for all > PP11 provides RO for all > > Today PP10 is used for RW pages and PP11 for RO pages. This patch > modifies page protection to PP01 for RW pages. > > Then segment registers are set to Ku 0 and Ks 1. When kernel needs > to write to RW pages, the associated segment register is changed to > Ks 0 in order to allow write access to the kernel. > > In order to avoid having the read all segment registers when > locking/unlocking the access, some data is kept in the thread_struct > and saved on stack on exceptions. The field identifies both the > first unlocked segment and the first segment following the last > unlocked one. When no segment is unlocked, it contains value 0. > > Signed-off-by: Christophe Leroy Hey Christophe, I tried to test this and got a machine check after the kernel starts init. Vector: 700 (Program Check) at [ef0b5e70] pc: 0ca4 lr: b7e1a030 sp: ef0b5f30 msr: 81002 current = 0xef0b8000 pid = 1, comm = init Testing with mac99 model in qemu. - Russell
Re: [PATCH] powerpc: eeh: stop using do_gettimeofday()
On Sat, 2017-11-04 at 22:26 +0100, Arnd Bergmann wrote: > This interface is inefficient and deprecated because of the y2038 > overflow. > > ktime_get_seconds() is an appropriate replacement here, since it > has sufficient granularity but is more efficient and uses monotonic > time. > > Signed-off-by: Arnd Bergmann <a...@arndb.de> Acked-by: Russell Currey <rus...@russell.cc>
Re: [PATCH] powerpc: eeh: stop using do_gettimeofday()
On Sat, 2017-11-04 at 22:26 +0100, Arnd Bergmann wrote: > This interface is inefficient and deprecated because of the y2038 > overflow. > > ktime_get_seconds() is an appropriate replacement here, since it > has sufficient granularity but is more efficient and uses monotonic > time. > > Signed-off-by: Arnd Bergmann Acked-by: Russell Currey
Re: qustion about eeh_add_virt_device
On Sun, 2017-08-13 at 16:54 +0200, Julia Lawall wrote:
> Hello,
Hello, sorry for the delayed response.
>
> At the suggestion of Christoph Hellwig, I am working on inlining the
> functions stored in the err_handler field of a pci_driver structure into
> the pci_driver structure itself. A number of functions in the file
> arch/powerpc/kernel/eeh_driver.c have code like:
>
> if (!driver->err_handler ||
> !driver->err_handler->error_detected) {
> eeh_pcid_put(dev);
> return NULL;
> }
>
> This I would just convert to:
>
> if (!driver->error_detected) {
> eeh_pcid_put(dev);
> return NULL;
> }
>
> But I am not sure what is best to do about eeh_add_virt_device, which
> contains:
>
> if (driver->err_handler)
> return NULL;
>
> Should I try to find a subfield of the err_handler that is guaranteed to
> be there if anything is there? Or could the test just be dropped, leaving
> a direct return NULL?
I believe the test can be dropped.
- Russell
>
> thanks,
> julia
Re: qustion about eeh_add_virt_device
On Sun, 2017-08-13 at 16:54 +0200, Julia Lawall wrote:
> Hello,
Hello, sorry for the delayed response.
>
> At the suggestion of Christoph Hellwig, I am working on inlining the
> functions stored in the err_handler field of a pci_driver structure into
> the pci_driver structure itself. A number of functions in the file
> arch/powerpc/kernel/eeh_driver.c have code like:
>
> if (!driver->err_handler ||
> !driver->err_handler->error_detected) {
> eeh_pcid_put(dev);
> return NULL;
> }
>
> This I would just convert to:
>
> if (!driver->error_detected) {
> eeh_pcid_put(dev);
> return NULL;
> }
>
> But I am not sure what is best to do about eeh_add_virt_device, which
> contains:
>
> if (driver->err_handler)
> return NULL;
>
> Should I try to find a subfield of the err_handler that is guaranteed to
> be there if anything is there? Or could the test just be dropped, leaving
> a direct return NULL?
I believe the test can be dropped.
- Russell
>
> thanks,
> julia
Re: [v2,2/2] reset: Add basic single-register reset driver
On Tue, 2017-05-30 at 15:38 +0930, Joel wrote: > This driver is a basic single-register reset controller driver that > supports clearing a single bit in a register. > > Signed-off-by: Joel Stanley <j...@jms.id.au> Acked-by: Russell Currey <rus...@russell.cc>
Re: [v2,2/2] reset: Add basic single-register reset driver
On Tue, 2017-05-30 at 15:38 +0930, Joel wrote: > This driver is a basic single-register reset controller driver that > supports clearing a single bit in a register. > > Signed-off-by: Joel Stanley Acked-by: Russell Currey
Re: [PATCH] powerpc/eeh/of: use builtin_platform_driver
On Wed, 2016-11-23 at 22:58 +0800, Geliang Tang wrote: > Use builtin_platform_driver() helper to simplify the code. > > Signed-off-by: Geliang Tang <geliangt...@gmail.com> > --- Acked-by: Russell Currey <rus...@russell.cc>
Re: [PATCH] powerpc/eeh/of: use builtin_platform_driver
On Wed, 2016-11-23 at 22:58 +0800, Geliang Tang wrote: > Use builtin_platform_driver() helper to simplify the code. > > Signed-off-by: Geliang Tang > --- Acked-by: Russell Currey
