* [Bug 205099] KASAN hit at raid6_pq: BUG: Unable to handle kernel data access at 0x00f0fd0d
From: bugzilla-daemon @ 2020-02-26 16:17 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-205099-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=205099
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #286395|0 |1
is obsolete| |
Attachment #286873|0 |1
is obsolete| |
Attachment #286927|0 |1
is obsolete| |
Attachment #286931|0 |1
is obsolete| |
--- Comment #27 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 287625
--> https://bugzilla.kernel.org/attachment.cgi?id=287625&action=edit
dmesg (5.6-rc3+, KASAN_VMALLOC + VMAP_STACK + CONFIG_THREAD_SHIFT=14 + INLINE
KASAN, PowerMac G4 DP)
And still the same phenomenon that I only get this hit with OUTLINE KASAN but
not with INLINE KASAN (+ CONFIG_THREAD_SHIFT=14).
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 205283] BUG: KASAN: global-out-of-bounds in _copy_to_iter+0x3d4/0x5a8
From: bugzilla-daemon @ 2020-02-26 16:37 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-205283-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=205283
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |CODE_FIX
--- Comment #17 from Erhard F. (erhard_f@mailbox.org) ---
Re-tested with 5.6-rc3 + KASAN_VMALLOC + VMAP_STACK + THREAD_SHIFT=14 + INLINE
KASAN. Works now, thanks!
--
You are receiving this mail because:
You are watching someone on the CC list of the bug.
^ permalink raw reply
* Re: [PATCH V2 2/4] mm/vma: Make vma_is_accessible() available for general use
From: Vlastimil Babka @ 2020-02-26 17:39 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, linux-kernel
Cc: Rich Felker, Yoshinori Sato, Paul Burton, linux-sh,
Peter Zijlstra, Dave Hansen, linux-mips, Guo Ren, linuxppc-dev,
linux-m68k, Ingo Molnar, Geert Uytterhoeven, Ralf Baechle,
Andy Lutomirski, Paul Mackerras, Thomas Gleixner, Steven Rostedt,
Andrew Morton, Mel Gorman
In-Reply-To: <1582520593-30704-3-git-send-email-anshuman.khandual@arm.com>
On 2/24/20 6:03 AM, Anshuman Khandual wrote:
> Lets move vma_is_accessible() helper to include/linux/mm.h which makes it
> available for general use. While here, this replaces all remaining open
> encodings for VMA access check with vma_is_accessible().
>
> Cc: Guo Ren <guoren@kernel.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-m68k@lists.linux-m68k.org
> Cc: linux-mips@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-sh@vger.kernel.org
> Cc: linux-mm@kvack.org
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Acked-by: Guo Ren <guoren@kernel.org>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz
^ permalink raw reply
* Re: [PATCH V2 3/4] mm/vma: Replace all remaining open encodings with is_vm_hugetlb_page()
From: Vlastimil Babka @ 2020-02-26 17:40 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm, linux-kernel
Cc: kvm-ppc, linux-arch, Arnaldo Carvalho de Melo, Arnd Bergmann,
linuxppc-dev, Nick Piggin, Peter Zijlstra, Ingo Molnar,
Alexander Viro, Aneesh Kumar K.V, linux-fsdevel, Andrew Morton,
Will Deacon
In-Reply-To: <1582520593-30704-4-git-send-email-anshuman.khandual@arm.com>
On 2/24/20 6:03 AM, Anshuman Khandual wrote:
> This replaces all remaining open encodings with is_vm_hugetlb_page().
>
> Cc: Paul Mackerras <paulus@ozlabs.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Will Deacon <will@kernel.org>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Nick Piggin <npiggin@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: kvm-ppc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-fsdevel@vger.kernel.org
> Cc: linux-arch@vger.kernel.org
> Cc: linux-mm@kvack.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
Meh, why is there _page in the function's name... but too many users to bother
changing it now, I guess.
Acked-by: Vlastimil Babka <vbabka@suse.cz
^ permalink raw reply
* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Christopher Lameter @ 2020-02-26 18:25 UTC (permalink / raw)
To: Michal Hocko
Cc: Sachin Sant, Pekka Enberg, Kirill Tkhai, Linux-Next Mailing List,
David Rientjes, Joonsoo Kim, linuxppc-dev
In-Reply-To: <20200224085812.GB22443@dhcp22.suse.cz>
On Mon, 24 Feb 2020, Michal Hocko wrote:
> Hmm, nasty. Is there any reason why kmalloc_node behaves differently
> from the page allocator?
The page allocator will do the same thing if you pass GFP_THISNODE and
insist on allocating memory from a node that does not exist.
> > > A short summary. kmalloc_node blows up when trying to allocate from a
> > > memory less node.
> >
> > Use kmalloc instead? And set a memory allocation policy?
>
> The current code (memcg_expand_one_shrinker_map resp. memcg_alloc_shrinker_maps)
> already use kvmalloc. Kirill's patch wanted to make those data structure
> on the respective node and kvmalloc_node sounded like the right thing to
> do. It comes as a surprise that the kernel simply blows up on a memory
> less node rather than falling back to a close node gracefully. I suspect
> this already happens when the target node is out of memory, right?
No. If the target node is out of memory then direct reclaim is going to be
invovked.
> How would a memory allocation policy help in this case btw.?
It would allow fallback to other nodes.
^ permalink raw reply
* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Michal Hocko @ 2020-02-26 18:41 UTC (permalink / raw)
To: Christopher Lameter
Cc: Sachin Sant, Pekka Enberg, Kirill Tkhai, Linux-Next Mailing List,
David Rientjes, Joonsoo Kim, linuxppc-dev
In-Reply-To: <alpine.DEB.2.21.2002261823270.8012@www.lameter.com>
On Wed 26-02-20 18:25:28, Cristopher Lameter wrote:
> On Mon, 24 Feb 2020, Michal Hocko wrote:
>
> > Hmm, nasty. Is there any reason why kmalloc_node behaves differently
> > from the page allocator?
>
> The page allocator will do the same thing if you pass GFP_THISNODE and
> insist on allocating memory from a node that does not exist.
I do not think that the page allocator would blow up even with
GFP_THISNODE. The allocation would just fail on memory less node.
Besides that kmalloc_node shouldn't really have an implicit GFP_THISNODE
semantic right? At least I do not see anything like that documented
anywhere.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Christopher Lameter @ 2020-02-26 18:44 UTC (permalink / raw)
To: Michal Hocko
Cc: Sachin Sant, Pekka Enberg, Kirill Tkhai, Linux-Next Mailing List,
David Rientjes, Joonsoo Kim, linuxppc-dev
In-Reply-To: <20200226184152.GQ3771@dhcp22.suse.cz>
On Wed, 26 Feb 2020, Michal Hocko wrote:
> Besides that kmalloc_node shouldn't really have an implicit GFP_THISNODE
> semantic right? At least I do not see anything like that documented
> anywhere.
Kmalloc_node does not support memory policies etc. Only kmalloc does.
kmalloc_node is mostly used by subsystems that have determined the active
nodes and want a targeted allocation on those nodes.
^ permalink raw reply
* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Michal Hocko @ 2020-02-26 19:01 UTC (permalink / raw)
To: Christopher Lameter
Cc: Sachin Sant, Pekka Enberg, Kirill Tkhai, Linux-Next Mailing List,
David Rientjes, Joonsoo Kim, linuxppc-dev
In-Reply-To: <alpine.DEB.2.21.2002261843200.8915@www.lameter.com>
On Wed 26-02-20 18:44:13, Cristopher Lameter wrote:
> On Wed, 26 Feb 2020, Michal Hocko wrote:
>
> > Besides that kmalloc_node shouldn't really have an implicit GFP_THISNODE
> > semantic right? At least I do not see anything like that documented
> > anywhere.
>
> Kmalloc_node does not support memory policies etc. Only kmalloc does.
> kmalloc_node is mostly used by subsystems that have determined the active
> nodes and want a targeted allocation on those nodes.
I am sorry but I have hard time to follow your responses here. They open
more questions than they answer for me. The primary point here is that
kmalloc_node on a memory less node blows up and panics the kernel. I
strongly believe this is a bug. We cannot really make all callers of
kmalloc_node and co. to be hotplug aware.
Another question is the semantic of kmalloc_node when the node cannot
satisfy the request. I have always thought that the allocation would
simply fall back to any other node unless __GFP_THISNODE is explicitly
specified.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3 04/27] ocxl: Remove unnecessary externs
From: Greg Kurz @ 2020-02-26 14:20 UTC (permalink / raw)
To: 'Baoquan He'
Cc: 'Madhavan Srinivasan', 'Alexey Kardashevskiy',
'Mahesh Salgaonkar', 'Masahiro Yamada',
'Oliver O'Halloran', 'Mauro Carvalho Chehab',
'Ira Weiny', 'Thomas Gleixner',
'Rob Herring', 'Dave Jiang', linux-nvdimm,
'Aneesh Kumar K . V', 'Krzysztof Kozlowski',
'Anju T Sudhakar', Alastair D'Silva,
'Andrew Donnellan', 'Arnd Bergmann',
'Nicholas Piggin', 'Cédric Le Goater',
'Dan Williams', 'Hari Bathini',
'Alastair D'Silva', linux-mm,
'Greg Kroah-Hartman', linux-kernel,
'Vishal Verma', 'Frederic Barrat',
'Paul Mackerras', 'Andrew Morton', linuxppc-dev,
'David S. Miller'
In-Reply-To: <20200226141523.GI4937@MiWiFi-R3L-srv>
On Wed, 26 Feb 2020 22:15:23 +0800
'Baoquan He' <bhe@redhat.com> wrote:
> On 02/26/20 at 10:01am, Greg Kurz wrote:
> > On Wed, 26 Feb 2020 19:26:34 +1100
> > "Alastair D'Silva" <alastair@d-silva.org> wrote:
> >
> > > > -----Original Message-----
> > > > From: Baoquan He <bhe@redhat.com>
> > > > Sent: Wednesday, 26 February 2020 7:15 PM
> > > > To: Alastair D'Silva <alastair@au1.ibm.com>
> > > > Cc: alastair@d-silva.org; Aneesh Kumar K . V
> > > > <aneesh.kumar@linux.ibm.com>; Oliver O'Halloran <oohall@gmail.com>;
> > > > Benjamin Herrenschmidt <benh@kernel.crashing.org>; Paul Mackerras
> > > > <paulus@samba.org>; Michael Ellerman <mpe@ellerman.id.au>; Frederic
> > > > Barrat <fbarrat@linux.ibm.com>; Andrew Donnellan <ajd@linux.ibm.com>;
> > > > Arnd Bergmann <arnd@arndb.de>; Greg Kroah-Hartman
> > > > <gregkh@linuxfoundation.org>; Dan Williams <dan.j.williams@intel.com>;
> > > > Vishal Verma <vishal.l.verma@intel.com>; Dave Jiang
> > > > <dave.jiang@intel.com>; Ira Weiny <ira.weiny@intel.com>; Andrew Morton
> > > > <akpm@linux-foundation.org>; Mauro Carvalho Chehab
> > > > <mchehab+samsung@kernel.org>; David S. Miller <davem@davemloft.net>;
> > > > Rob Herring <robh@kernel.org>; Anton Blanchard <anton@ozlabs.org>;
> > > > Krzysztof Kozlowski <krzk@kernel.org>; Mahesh Salgaonkar
> > > > <mahesh@linux.vnet.ibm.com>; Madhavan Srinivasan
> > > > <maddy@linux.vnet.ibm.com>; Cédric Le Goater <clg@kaod.org>; Anju T
> > > > Sudhakar <anju@linux.vnet.ibm.com>; Hari Bathini
> > > > <hbathini@linux.ibm.com>; Thomas Gleixner <tglx@linutronix.de>; Greg
> > > > Kurz <groug@kaod.org>; Nicholas Piggin <npiggin@gmail.com>; Masahiro
> > > > Yamada <yamada.masahiro@socionext.com>; Alexey Kardashevskiy
> > > > <aik@ozlabs.ru>; linux-kernel@vger.kernel.org; linuxppc-
> > > > dev@lists.ozlabs.org; linux-nvdimm@lists.01.org; linux-mm@kvack.org
> > > > Subject: Re: [PATCH v3 04/27] ocxl: Remove unnecessary externs
> > > >
> > > > On 02/21/20 at 02:26pm, Alastair D'Silva wrote:
> > > > > From: Alastair D'Silva <alastair@d-silva.org>
> > > > >
> > > > > Function declarations don't need externs, remove the existing ones so
> > > > > they are consistent with newer code
> > > > >
> > > > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > > > > ---
> > > > > arch/powerpc/include/asm/pnv-ocxl.h | 32 ++++++++++++++---------------
> > > > > include/misc/ocxl.h | 6 +++---
> > > > > 2 files changed, 18 insertions(+), 20 deletions(-)
> > > > >
> > > > > diff --git a/arch/powerpc/include/asm/pnv-ocxl.h
> > > > > b/arch/powerpc/include/asm/pnv-ocxl.h
> > > > > index 0b2a6707e555..b23c99bc0c84 100644
> > > > > --- a/arch/powerpc/include/asm/pnv-ocxl.h
> > > > > +++ b/arch/powerpc/include/asm/pnv-ocxl.h
> > > > > @@ -9,29 +9,27 @@
> > > > > #define PNV_OCXL_TL_BITS_PER_RATE 4
> > > > > #define PNV_OCXL_TL_RATE_BUF_SIZE
> > > > ((PNV_OCXL_TL_MAX_TEMPLATE+1) * PNV_OCXL_TL_BITS_PER_RATE / 8)
> > > > >
> > > > > -extern int pnv_ocxl_get_actag(struct pci_dev *dev, u16 *base, u16
> > > > *enabled,
> > > > > - u16 *supported);
> > > >
> > > > It works w or w/o extern when declare functions. Searching 'extern'
> > > > under include can find so many functions with 'extern' adding. Do we have
> > > a
> > > > explicit standard if we should add or remove 'exter' in function
> > > declaration?
> > > >
> > > > I have no objection to this patch, just want to make clear so that I can
> > > handle
> > > > it w/o confusion.
> > > >
> > > > Thanks
> > > > Baoquan
> > > >
> > >
> > > For the OpenCAPI driver, we have settled on not having 'extern' on
> > > functions.
> > >
> > > I don't think I've seen a standard that supports or refutes this, but it
> > > does not value add.
> > >
> >
> > FWIW this is a warning condition for checkpatch:
> >
> > $ ./scripts/checkpatch.pl --strict -f include/misc/ocxl.h
>
> Good to know, thanks.
>
> I didn't know checkpatch.pl can run on header file directly. Tried to
> check patch with '--strict -f', the below info doesn't appear. But it
Hmm... -f is to check a source file, not a patch... What did you try
exactly ?
> does give out below information when run on header file.
>
> >
> > [...]
> >
> > CHECK: extern prototypes should be avoided in .h files
> > #176: FILE: include/misc/ocxl.h:176:
> > +extern int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id);
> >
> > [...]
> >
>
^ permalink raw reply
* Re: [PATCH v3 10/13] powerpc/ptrace: split out ADV_DEBUG_REGS related functions.
From: kbuild test robot @ 2020-02-26 19:11 UTC (permalink / raw)
To: Christophe Leroy
Cc: mikey, kbuild-all, linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <06a9634b5602ef7121fdae1385c74e090b706e17.1582717645.git.christophe.leroy@c-s.fr>
[-- Attachment #1: Type: text/plain, Size: 4234 bytes --]
Hi Christophe,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.6-rc3 next-20200226]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/Reduce-ifdef-mess-in-ptrace/20200227-011143
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Note: the linux-review/Christophe-Leroy/Reduce-ifdef-mess-in-ptrace/20200227-011143 HEAD 14a582f53275332199397e25fe49bd2597d4cb19 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
arch/powerpc/kernel/ptrace/ptrace.c: In function 'ptrace_triggered':
>> arch/powerpc/kernel/ptrace/ptrace.c:46:2: error: implicit declaration of function 'modify_user_hw_breakpoint'; did you mean '__set_breakpoint'? [-Werror=implicit-function-declaration]
modify_user_hw_breakpoint(bp, &attr);
^~~~~~~~~~~~~~~~~~~~~~~~~
__set_breakpoint
cc1: all warnings being treated as errors
vim +46 arch/powerpc/kernel/ptrace/ptrace.c
57d28fcb6dde29 arch/powerpc/kernel/ptrace/ptrace.c Christophe Leroy 2020-02-26 31
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 32 #ifdef CONFIG_HAVE_HW_BREAKPOINT
a8b0ca17b80e92 arch/powerpc/kernel/ptrace.c Peter Zijlstra 2011-06-27 33 void ptrace_triggered(struct perf_event *bp,
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 34 struct perf_sample_data *data, struct pt_regs *regs)
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 35 {
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 36 struct perf_event_attr attr;
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 37
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 38 /*
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 39 * Disable the breakpoint request here since ptrace has defined a
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 40 * one-shot behaviour for breakpoint exceptions in PPC64.
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 41 * The SIGTRAP signal is generated automatically for us in do_dabr().
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 42 * We don't have to do anything about that here
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 43 */
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 44 attr = bp->attr;
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 45 attr.disabled = true;
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 @46 modify_user_hw_breakpoint(bp, &attr);
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 47 }
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 48 #endif /* CONFIG_HAVE_HW_BREAKPOINT */
5aae8a53708025 arch/powerpc/kernel/ptrace.c K.Prasad 2010-06-15 49
:::::: The code at line 46 was first introduced by commit
:::::: 5aae8a53708025d4e718f0d2e7c2f766779ddc71 powerpc, hw_breakpoints: Implement hw_breakpoints for 64-bit server processors
:::::: TO: K.Prasad <prasad@linux.vnet.ibm.com>
:::::: CC: Paul Mackerras <paulus@samba.org>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25741 bytes --]
^ permalink raw reply
* Re: [PATCH] ima: add a new CONFIG for loading arch-specific policies
From: Lakshmi Ramasubramanian @ 2020-02-26 19:21 UTC (permalink / raw)
To: Nayna Jain, linux-integrity, linuxppc-dev, linux-efi, linux-s390
Cc: linux-kernel, zohar, Philipp Rudo, Martin Schwidefsky,
Ard Biesheuvel
In-Reply-To: <1582744207-25969-1-git-send-email-nayna@linux.ibm.com>
Hi Nayna,
> +
> +config IMA_SECURE_AND_OR_TRUSTED_BOOT
> + bool
> + depends on IMA
> + depends on IMA_ARCH_POLICY
> + default n
> + help
> + This option is selected by architectures to enable secure and/or
> + trusted boot based on IMA runtime policies.
>
Why is the default for this new config "n"?
Is there any reason to not turn on this config if both IMA and
IMA_ARCH_POLICY are set to y?
thanks,
-lakshmi
^ permalink raw reply
* [PATCH] ima: add a new CONFIG for loading arch-specific policies
From: Nayna Jain @ 2020-02-26 19:10 UTC (permalink / raw)
To: linux-integrity, linuxppc-dev, linux-efi, linux-s390
Cc: Nayna Jain, linux-kernel, zohar, Philipp Rudo, Martin Schwidefsky,
Ard Biesheuvel
Every time a new architecture defines the IMA architecture specific
functions - arch_ima_get_secureboot() and arch_ima_get_policy(), the IMA
include file needs to be updated. To avoid this "noise", this patch
defines a new IMA Kconfig IMA_SECURE_AND_OR_TRUSTED_BOOT option, allowing
the different architectures to select it.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Philipp Rudo <prudo@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/Kconfig | 2 +-
arch/s390/Kconfig | 1 +
arch/x86/Kconfig | 1 +
include/linux/ima.h | 3 +--
security/integrity/ima/Kconfig | 9 +++++++++
5 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 497b7d0b2d7e..b8ce1b995633 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -246,6 +246,7 @@ config PPC
select SYSCTL_EXCEPTION_TRACE
select THREAD_INFO_IN_TASK
select VIRT_TO_BUS if !PPC64
+ select IMA_SECURE_AND_OR_TRUSTED_BOOT if PPC_SECURE_BOOT
#
# Please keep this list sorted alphabetically.
#
@@ -978,7 +979,6 @@ config PPC_SECURE_BOOT
prompt "Enable secure boot support"
bool
depends on PPC_POWERNV
- depends on IMA_ARCH_POLICY
help
Systems with firmware secure boot enabled need to define security
policies to extend secure boot to the OS. This config allows a user
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 8abe77536d9d..90ff3633ade6 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -195,6 +195,7 @@ config S390
select ARCH_HAS_FORCE_DMA_UNENCRYPTED
select SWIOTLB
select GENERIC_ALLOCATOR
+ select IMA_SECURE_AND_OR_TRUSTED_BOOT
config SCHED_OMIT_FRAME_POINTER
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index beea77046f9b..cafa66313fe2 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -230,6 +230,7 @@ config X86
select VIRT_TO_BUS
select X86_FEATURE_NAMES if PROC_FS
select PROC_PID_ARCH_STATUS if PROC_FS
+ select IMA_SECURE_AND_OR_TRUSTED_BOOT if EFI
config INSTRUCTION_DECODER
def_bool y
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 1659217e9b60..aefe758f4466 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -30,8 +30,7 @@ extern void ima_kexec_cmdline(const void *buf, int size);
extern void ima_add_kexec_buffer(struct kimage *image);
#endif
-#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
- || defined(CONFIG_PPC_SECURE_BOOT)
+#ifdef CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT
extern bool arch_ima_get_secureboot(void);
extern const char * const *arch_get_ima_policy(void);
#else
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 3f3ee4e2eb0d..d17972aa413a 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -327,3 +327,12 @@ config IMA_QUEUE_EARLY_BOOT_KEYS
depends on IMA_MEASURE_ASYMMETRIC_KEYS
depends on SYSTEM_TRUSTED_KEYRING
default y
+
+config IMA_SECURE_AND_OR_TRUSTED_BOOT
+ bool
+ depends on IMA
+ depends on IMA_ARCH_POLICY
+ default n
+ help
+ This option is selected by architectures to enable secure and/or
+ trusted boot based on IMA runtime policies.
--
2.18.1
^ permalink raw reply related
* Re: [PATCH] ima: add a new CONFIG for loading arch-specific policies
From: Mimi Zohar @ 2020-02-26 20:36 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, Nayna Jain, linux-integrity,
linuxppc-dev, linux-efi, linux-s390
Cc: Martin Schwidefsky, Philipp Rudo, Ard Biesheuvel, linux-kernel
In-Reply-To: <94fe39a9-db9e-211d-d9b7-4cfe1a270e6f@linux.microsoft.com>
On Wed, 2020-02-26 at 11:21 -0800, Lakshmi Ramasubramanian wrote:
> Hi Nayna,
>
> > +
> > +config IMA_SECURE_AND_OR_TRUSTED_BOOT
> > + bool
> > + depends on IMA
> > + depends on IMA_ARCH_POLICY
> > + default n
> > + help
> > + This option is selected by architectures to enable secure and/or
> > + trusted boot based on IMA runtime policies.
> >
>
> Why is the default for this new config "n"?
> Is there any reason to not turn on this config if both IMA and
> IMA_ARCH_POLICY are set to y?
Good catch. Having "IMA_SECURE_AND_OR_TRUSTED_BOOT" depend on
"IMA_ARCH_POLICY" doesn't make sense. "IMA_ARCH_POLICY" needs to be
selected.
thanks,
Mimi
^ permalink raw reply
* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: David Rientjes @ 2020-02-26 20:31 UTC (permalink / raw)
To: Michal Hocko
Cc: Sachin Sant, Pekka Enberg, Kirill Tkhai, Linux-Next Mailing List,
Christopher Lameter, linuxppc-dev, Joonsoo Kim
In-Reply-To: <20200226190147.GR3771@dhcp22.suse.cz>
On Wed, 26 Feb 2020, Michal Hocko wrote:
> On Wed 26-02-20 18:44:13, Cristopher Lameter wrote:
> > On Wed, 26 Feb 2020, Michal Hocko wrote:
> >
> > > Besides that kmalloc_node shouldn't really have an implicit GFP_THISNODE
> > > semantic right? At least I do not see anything like that documented
> > > anywhere.
> >
> > Kmalloc_node does not support memory policies etc. Only kmalloc does.
> > kmalloc_node is mostly used by subsystems that have determined the active
> > nodes and want a targeted allocation on those nodes.
>
> I am sorry but I have hard time to follow your responses here. They open
> more questions than they answer for me. The primary point here is that
> kmalloc_node on a memory less node blows up and panics the kernel. I
> strongly believe this is a bug. We cannot really make all callers of
> kmalloc_node and co. to be hotplug aware.
>
> Another question is the semantic of kmalloc_node when the node cannot
> satisfy the request. I have always thought that the allocation would
> simply fall back to any other node unless __GFP_THISNODE is explicitly
> specified.
>
Am I right in classifying this as a trade-off between an
unlikely(!node_state(nid, N_MEMORY)) directly in kmalloc_node() vs fixing
up a caller passing a memoryless nid?
Seems like we wouldn't want to penalize kmalloc_node() for making such a
check for 99.99% of allocators that don't need it and would rather do a
node_to_mem_node(nid) or numa_mem_id() in the caller?
^ permalink raw reply
* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Michal Hocko @ 2020-02-26 20:52 UTC (permalink / raw)
To: David Rientjes
Cc: Sachin Sant, Pekka Enberg, Kirill Tkhai, Linux-Next Mailing List,
Christopher Lameter, linuxppc-dev, Joonsoo Kim
In-Reply-To: <alpine.DEB.2.21.2002261228060.208847@chino.kir.corp.google.com>
On Wed 26-02-20 12:31:56, David Rientjes wrote:
> On Wed, 26 Feb 2020, Michal Hocko wrote:
>
> > On Wed 26-02-20 18:44:13, Cristopher Lameter wrote:
> > > On Wed, 26 Feb 2020, Michal Hocko wrote:
> > >
> > > > Besides that kmalloc_node shouldn't really have an implicit GFP_THISNODE
> > > > semantic right? At least I do not see anything like that documented
> > > > anywhere.
> > >
> > > Kmalloc_node does not support memory policies etc. Only kmalloc does.
> > > kmalloc_node is mostly used by subsystems that have determined the active
> > > nodes and want a targeted allocation on those nodes.
> >
> > I am sorry but I have hard time to follow your responses here. They open
> > more questions than they answer for me. The primary point here is that
> > kmalloc_node on a memory less node blows up and panics the kernel. I
> > strongly believe this is a bug. We cannot really make all callers of
> > kmalloc_node and co. to be hotplug aware.
> >
> > Another question is the semantic of kmalloc_node when the node cannot
> > satisfy the request. I have always thought that the allocation would
> > simply fall back to any other node unless __GFP_THISNODE is explicitly
> > specified.
> >
>
> Am I right in classifying this as a trade-off between an
> unlikely(!node_state(nid, N_MEMORY)) directly in kmalloc_node() vs fixing
> up a caller passing a memoryless nid?
The thing is that any check for node online/populated followed by the
allocation is inherently racy without using memory hotplug locking
around that and I am pretty sure this is a step into a wrong direction.
Is there any problem to initialize slub internal data structures for all
possible nodes? This wouldn't require any checks into hot paths.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [Bug 206525] BUG: KASAN: stack-out-of-bounds in test_bit+0x30/0x44 (kernel 5.6-rc1)
From: bugzilla-daemon @ 2020-02-26 21:44 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206525-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206525
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |CODE_FIX
--- Comment #7 from Erhard F. (erhard_f@mailbox.org) ---
Fix landed in 5.6-rc3, works now as expected. Thanks!
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Vlastimil Babka @ 2020-02-26 21:45 UTC (permalink / raw)
To: Michal Hocko, Christopher Lameter
Cc: Sachin Sant, Pekka Enberg, Kirill Tkhai, Linux-Next Mailing List,
David Rientjes, Joonsoo Kim, linuxppc-dev
In-Reply-To: <20200226184152.GQ3771@dhcp22.suse.cz>
On 2/26/20 7:41 PM, Michal Hocko wrote:
> On Wed 26-02-20 18:25:28, Cristopher Lameter wrote:
>> On Mon, 24 Feb 2020, Michal Hocko wrote:
>>
>>> Hmm, nasty. Is there any reason why kmalloc_node behaves differently
>>> from the page allocator?
>>
>> The page allocator will do the same thing if you pass GFP_THISNODE and
>> insist on allocating memory from a node that does not exist.
>
> I do not think that the page allocator would blow up even with
> GFP_THISNODE. The allocation would just fail on memory less node.
>
> Besides that kmalloc_node shouldn't really have an implicit GFP_THISNODE
> semantic right? At least I do not see anything like that documented
> anywhere.
Seems like SLAB at least behaves like the page allocator. See
____cache_alloc_node() where it basically does:
page = cache_grow_begin(cachep, gfp_exact_node(flags), nodeid);
...
if (!page)
fallback_alloc(cachep, flags)
gfp_exact_node() adds __GFP_THISNODE among other things, so the initial
attempt does try to stick only to the given node. But fallback_alloc()
doesn't. In fact, even if kmalloc_node() was called with __GFP_THISNODE
then it wouldn't work as intended, as fallback_alloc() doesn't get the
nodeid, but instead will use numa_mem_id(). That part could probably be
improved.
SLUB's ___slab_alloc() has for example this:
if (node != NUMA_NO_NODE && !node_present_pages(node))
searchnode = node_to_mem_node(node);
That's from Joonsoo's 2014 commit a561ce00b09e ("slub: fall back to
node_to_mem_node() node if allocating on memoryless node"), suggesting
that the scenario in this bug report should work. Perhaps it just got
broken unintentionally later.
And AFAICS the whole path leading to alloc_slab_page() also doesn't add
__GFP_THISNODE, but will keep it if caller passed it, and ultimately it
does:
if (node == NUMA_NO_NODE)
page = alloc_pages(flags, order);
else
page = __alloc_pages_node(node, flags, order);
So yeah looks like SLUB's kmalloc_node() is supposed to behave like the
page allocator's __alloc_pages_node() and respect __GFP_THISNODE but not
enforce it by itself. There's probably just some missing data structure
initialization somewhere right now for memoryless nodes.
^ permalink raw reply
* Re: [PATCH v5 3/8] powerpc/mm/ptdump: debugfs handler for W+X checks at runtime
From: Kees Cook @ 2020-02-26 21:52 UTC (permalink / raw)
To: Russell Currey
Cc: ajd, kernel-hardening, jniethe5, npiggin, joel, linuxppc-dev, dja
In-Reply-To: <20200226063551.65363-4-ruscur@russell.cc>
On Wed, Feb 26, 2020 at 05:35:46PM +1100, Russell Currey wrote:
> Very rudimentary, just
>
> echo 1 > [debugfs]/check_wx_pages
>
> and check the kernel log. Useful for testing strict module RWX.
>
> Updated the Kconfig entry to reflect this.
Oh, I like this! This would be handy to have on all architectures.
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
>
> Also fixed a typo.
>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
> ---
> arch/powerpc/Kconfig.debug | 6 ++++--
> arch/powerpc/mm/ptdump/ptdump.c | 21 ++++++++++++++++++++-
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> index 0b063830eea8..e37960ef68c6 100644
> --- a/arch/powerpc/Kconfig.debug
> +++ b/arch/powerpc/Kconfig.debug
> @@ -370,7 +370,7 @@ config PPC_PTDUMP
> If you are unsure, say N.
>
> config PPC_DEBUG_WX
> - bool "Warn on W+X mappings at boot"
> + bool "Warn on W+X mappings at boot & enable manual checks at runtime"
> depends on PPC_PTDUMP && STRICT_KERNEL_RWX
> help
> Generate a warning if any W+X mappings are found at boot.
> @@ -384,7 +384,9 @@ config PPC_DEBUG_WX
> of other unfixed kernel bugs easier.
>
> There is no runtime or memory usage effect of this option
> - once the kernel has booted up - it's a one time check.
> + once the kernel has booted up, it only automatically checks once.
> +
> + Enables the "check_wx_pages" debugfs entry for checking at runtime.
>
> If in doubt, say "Y".
>
> diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
> index 206156255247..a15e19a3b14e 100644
> --- a/arch/powerpc/mm/ptdump/ptdump.c
> +++ b/arch/powerpc/mm/ptdump/ptdump.c
> @@ -4,7 +4,7 @@
> *
> * This traverses the kernel pagetables and dumps the
> * information about the used sections of memory to
> - * /sys/kernel/debug/kernel_pagetables.
> + * /sys/kernel/debug/kernel_page_tables.
> *
> * Derived from the arm64 implementation:
> * Copyright (c) 2014, The Linux Foundation, Laura Abbott.
> @@ -413,6 +413,25 @@ void ptdump_check_wx(void)
> else
> pr_info("Checked W+X mappings: passed, no W+X pages found\n");
> }
> +
> +static int check_wx_debugfs_set(void *data, u64 val)
> +{
> + if (val != 1ULL)
> + return -EINVAL;
> +
> + ptdump_check_wx();
> +
> + return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(check_wx_fops, NULL, check_wx_debugfs_set, "%llu\n");
> +
> +static int ptdump_check_wx_init(void)
> +{
> + return debugfs_create_file("check_wx_pages", 0200, NULL,
> + NULL, &check_wx_fops) ? 0 : -ENOMEM;
> +}
> +device_initcall(ptdump_check_wx_init);
> #endif
>
> static int ptdump_init(void)
> --
> 2.25.1
>
--
Kees Cook
^ permalink raw reply
* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Vlastimil Babka @ 2020-02-26 22:29 UTC (permalink / raw)
To: Michal Hocko, Christopher Lameter
Cc: Sachin Sant, Pekka Enberg, Kirill Tkhai, Linux-Next Mailing List,
David Rientjes, Joonsoo Kim, linuxppc-dev
In-Reply-To: <c412ee69-80f9-b013-67d4-3b0a2f6aff7f@suse.cz>
On 2/26/20 10:45 PM, Vlastimil Babka wrote:
>
>
> if (node == NUMA_NO_NODE)
> page = alloc_pages(flags, order);
> else
> page = __alloc_pages_node(node, flags, order);
>
> So yeah looks like SLUB's kmalloc_node() is supposed to behave like the
> page allocator's __alloc_pages_node() and respect __GFP_THISNODE but not
> enforce it by itself. There's probably just some missing data structure
> initialization somewhere right now for memoryless nodes.
Upon more digging, I think the problem could manifest if
node_to_mem_node(0) (_node_numa_mem_[0]) returned 0 instead of 1,
because it wasn't initialized properly for a memoryless node. Can you
e.g. print it somewhere?
^ permalink raw reply
* Re: [PATCH v2 0/3] Fix some incompatibilites between KASAN and FORTIFY_SOURCE
From: Daniel Axtens @ 2020-02-26 23:48 UTC (permalink / raw)
To: linux-kernel, linux-mm, kasan-dev
Cc: linux-s390, linux-xtensa, x86, linux-arm-kernel, linuxppc-dev,
dvyukov
In-Reply-To: <20200116062625.32692-1-dja@axtens.net>
Daniel Axtens <dja@axtens.net> writes:
> 3 KASAN self-tests fail on a kernel with both KASAN and FORTIFY_SOURCE:
> memchr, memcmp and strlen. I have observed this on x86 and powerpc.
>
> When FORTIFY_SOURCE is on, a number of functions are replaced with
> fortified versions, which attempt to check the sizes of the
> operands. However, these functions often directly invoke __builtin_foo()
> once they have performed the fortify check.
>
> This breaks things in 2 ways:
>
> - the three function calls are technically dead code, and can be
> eliminated. When __builtin_ versions are used, the compiler can detect
> this.
>
> - Using __builtins may bypass KASAN checks if the compiler decides to
> inline it's own implementation as sequence of instructions, rather than
> emit a function call that goes out to a KASAN-instrumented
> implementation.
>
> The patches address each reason in turn. Finally, test_memcmp used a
> stack array without explicit initialisation, which can sometimes break
> too, so fix that up.
Hi all,
It doesn't look like this has been picked up yet. Is there anything I
can do to help things along?
Regards,
Daniel
>
> v2: - some cleanups, don't mess with arch code as I missed some wrinkles.
> - add stack array init (patch 3)
>
> Daniel Axtens (3):
> kasan: stop tests being eliminated as dead code with FORTIFY_SOURCE
> string.h: fix incompatibility between FORTIFY_SOURCE and KASAN
> kasan: initialise array in kasan_memcmp test
>
> include/linux/string.h | 60 +++++++++++++++++++++++++++++++++---------
> lib/test_kasan.c | 32 +++++++++++++---------
> 2 files changed, 68 insertions(+), 24 deletions(-)
>
> --
> 2.20.1
^ permalink raw reply
* Re: [PATCH v3 07/14] powerpc/traps: Check for prefixed instructions in facility_unavailable_exception()
From: Jordan Niethe @ 2020-02-26 23:52 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Alistair Popple, Daniel Axtens, linuxppc-dev, Balamuruhan S
In-Reply-To: <1582699744.9jdw100hst.astroid@bobo.none>
On Wed, Feb 26, 2020 at 5:53 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Jordan Niethe's on February 26, 2020 2:07 pm:
> > If prefixed instructions are made unavailable by the [H]FSCR, attempting
> > to use them will cause a facility unavailable exception. Add "PREFIX" to
> > the facility_strings[].
> >
> > Currently there are no prefixed instructions that are actually emulated
> > by emulate_instruction() within facility_unavailable_exception().
> > However, when caused by a prefixed instructions the SRR1 PREFIXED bit is
> > set. Prepare for dealing with emulated prefixed instructions by checking
> > for this bit.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
>
> Oh you've got it here, I would just squash this together with the first
> patch.
Sure, I'll put them together. When you mentioned a couple more things
to do in traps.c, was it just this? Or is there still more to be done
adding an FSCR?
>
> Thanks,
> Nick
^ permalink raw reply
* Re: [PATCH v3 08/14] powerpc/xmon: Remove store_inst() for patch_instruction()
From: Jordan Niethe @ 2020-02-26 23:54 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Alistair Popple, Daniel Axtens, linuxppc-dev, Balamuruhan S
In-Reply-To: <1582699938.333f3hx0ka.astroid@bobo.none>
On Wed, Feb 26, 2020 at 6:04 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Jordan Niethe's on February 26, 2020 2:07 pm:
> > For modifying instructions in xmon, patch_instruction() can serve the
> > same role that store_inst() is performing with the advantage of not
> > being specific to xmon. In some places patch_instruction() is already
> > being using followed by store_inst(). In these cases just remove the
> > store_inst(). Otherwise replace store_inst() with patch_instruction().
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > arch/powerpc/xmon/xmon.c | 13 ++-----------
> > 1 file changed, 2 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index 897e512c6379..a673cf55641c 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -325,11 +325,6 @@ static inline void sync(void)
> > asm volatile("sync; isync");
> > }
> >
> > -static inline void store_inst(void *p)
> > -{
> > - asm volatile ("dcbst 0,%0; sync; icbi 0,%0; isync" : : "r" (p));
> > -}
> > -
> > static inline void cflush(void *p)
> > {
> > asm volatile ("dcbf 0,%0; icbi 0,%0" : : "r" (p));
> > @@ -882,8 +877,7 @@ static struct bpt *new_breakpoint(unsigned long a)
> > for (bp = bpts; bp < &bpts[NBPTS]; ++bp) {
> > if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
> > bp->address = a;
> > - bp->instr[1] = bpinstr;
> > - store_inst(&bp->instr[1]);
> > + patch_instruction(&bp->instr[1], bpinstr);
> > return bp;
> > }
> > }
> > @@ -913,7 +907,7 @@ static void insert_bpts(void)
> > bp->enabled = 0;
> > continue;
> > }
> > - store_inst(&bp->instr[0]);
> > + patch_instruction(&bp->instr[0], bp->instr[0]);
>
> Hmm that's a bit weird. Can you read instructions into a local variable
> first, do the checks on them, then patch them into their execution
> location?
I agree it is weird, local variables would be better.
>
> Otherwise, good cleanup.
>
> Thanks,
> Nick
^ permalink raw reply
* Re: [PATCH v3 09/14] powerpc/xmon: Add initial support for prefixed instructions
From: Jordan Niethe @ 2020-02-27 0:11 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Alistair Popple, Daniel Axtens, linuxppc-dev, Balamuruhan S
In-Reply-To: <1582700457.bqyiezlzr9.astroid@bobo.none>
On Wed, Feb 26, 2020 at 6:10 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Jordan Niethe's on February 26, 2020 2:07 pm:
> > A prefixed instruction is composed of a word prefix and a word suffix.
> > It does not make sense to be able to have a breakpoint on the suffix of
> > a prefixed instruction, so make this impossible.
> >
> > When leaving xmon_core() we check to see if we are currently at a
> > breakpoint. If this is the case, the breakpoint needs to be proceeded
> > from. Initially emulate_step() is tried, but if this fails then we need
> > to execute the saved instruction out of line. The NIP is set to the
> > address of bpt::instr[] for the current breakpoint. bpt::instr[]
> > contains the instruction replaced by the breakpoint, followed by a trap
> > instruction. After bpt::instr[0] is executed and we hit the trap we
> > enter back into xmon_bpt(). We know that if we got here and the offset
> > indicates we are at bpt::instr[1] then we have just executed out of line
> > so we can put the NIP back to the instruction after the breakpoint
> > location and continue on.
> >
> > Adding prefixed instructions complicates this as the bpt::instr[1] needs
> > to be used to hold the suffix. To deal with this make bpt::instr[] big
> > enough for three word instructions. bpt::instr[2] contains the trap,
> > and in the case of word instructions pad bpt::instr[1] with a noop.
> >
> > No support for disassembling prefixed instructions.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v2: Rename sufx to suffix
> > v3: - Just directly use PPC_INST_NOP
> > - Typo: plac -> place
> > - Rename read_inst() to mread_inst(). Do not have it call mread().
> > ---
> > arch/powerpc/xmon/xmon.c | 90 ++++++++++++++++++++++++++++++++++------
> > 1 file changed, 78 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index a673cf55641c..a73a35aa4a75 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -97,7 +97,8 @@ static long *xmon_fault_jmp[NR_CPUS];
> > /* Breakpoint stuff */
> > struct bpt {
> > unsigned long address;
> > - unsigned int instr[2];
> > + /* Prefixed instructions can not cross 64-byte boundaries */
> > + unsigned int instr[3] __aligned(64);
>
> This is pretty wild, I didn't realize xmon executes breakpoints out
> of line like this.
>
> IMO the break point entries here should correspond with a range of
> reserved bytes in .text so we patch instructions into normal executable
> pages rather than .data.
Would it make sense to use vmalloc_exec() and use that like we are
going to do in kprobes()?
>
> Anyway that's for patch.
>
> Thanks,
> Nick
^ permalink raw reply
* Re: [PATCH v3 11/14] powerpc/kprobes: Support kprobes on prefixed instructions
From: Jordan Niethe @ 2020-02-27 0:58 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Alistair Popple, Daniel Axtens, linuxppc-dev, Balamuruhan S
In-Reply-To: <1582700856.cbydlhx2wj.astroid@bobo.none>
On Wed, Feb 26, 2020 at 6:18 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Jordan Niethe's on February 26, 2020 2:07 pm:
> > @@ -136,11 +148,14 @@ int arch_prepare_kprobe(struct kprobe *p)
> > }
> >
> > if (!ret) {
> > - patch_instruction(p->ainsn.insn, *p->addr);
> > + patch_instruction(&p->ainsn.insn[0], p->addr[0]);
> > + if (IS_PREFIX(insn))
> > + patch_instruction(&p->ainsn.insn[1], p->addr[1]);
> > p->opcode = *p->addr;
>
> Not to single out this hunk or this patch even, but what do you reckon
> about adding an instruction data type, and then use that in all these
> call sites rather than adding the extra arg or doing the extra copy
> manually in each place depending on prefix?
>
> instrs_are_equal, get_user_instr, analyse_instr, patch_instruction,
> etc., would all take this new instr. Places that open code a memory
> access like your MCE change need some accessor
>
> instr = *(unsigned int *)(instr_addr);
> - if (!analyse_instr(&op, &tmp, instr, PPC_NO_SUFFIX)) {
> + if (IS_PREFIX(instr))
> + suffix = *(unsigned int *)(instr_addr + 4);
>
> Becomes
> read_instr(instr_addr, &instr);
> if (!analyse_instr(&op, &tmp, instr)) ...
>
> etc.
Daniel Axtens also talked about this and my reasons not to do so were
pretty unconvincing, so I started trying something like this. One
thing I have been wondering is how pervasive should the new type be.
Below is data type I have started using, which I think works
reasonably for replacing unsigned ints everywhere (like within
code-patching.c). In a few architecture independent places such as
uprobes which want to do ==, etc the union type does not work so well.
I will have the next revision of the series start using a type.
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
new file mode 100644
index 000000000000..50adb3dbdeb4
--- /dev/null
+++ b/arch/powerpc/include/asm/inst.h
@@ -0,0 +1,87 @@
+
+#ifndef _ASM_INST_H
+#define _ASM_INST_H
+
+#ifdef __powerpc64__
+
+/* 64 bit Instruction */
+
+typedef struct {
+ unsigned int prefix;
+ unsigned int suffix;
+} __packed ppc_prefixed_inst;
+
+typedef union ppc_inst {
+ unsigned int w;
+ ppc_prefixed_inst p;
+} ppc_inst;
+
+#define PPC_INST_IS_PREFIXED(inst) (((inst).w >> 26) == 1)
+#define PPC_INST_LEN(inst) (PPC_INST_IS_PREFIXED((inst)) ?
sizeof((inst).p) : sizeof((inst).w))
+
+#define PPC_INST_NEW_WORD(x) ((ppc_inst) { .w = (x) })
+#define PPC_INST_NEW_WORD_PAD(x) ((ppc_inst) { .p.prefix = (x),
.p.suffix = (0x60000000) })
+#define PPC_INST_NEW_PREFIXED(x, y) ((ppc_inst) { .p.prefix = (x),
.p.suffix = (y) })
+
+#define PPC_INST_WORD(x) ((x).w)
+#define PPC_INST_PREFIX(x) (x.p.prefix)
+#define PPC_INST_SUFFIX(x) (x.p.suffix)
+#define PPC_INST_EMPTY(x) (PPC_INST_WORD(x) == 0)
+
+#define DEREF_PPC_INST_PTR(ptr) \
+({ \
+ ppc_inst __inst; \
+ __inst.w = *(unsigned int *)(ptr); \
+ if (PPC_INST_IS_PREFIXED(__inst)) \
+ __inst.p = *(ppc_prefixed_inst *)(ptr); \
+ __inst; \
+})
+
+#define PPC_INST_NEXT(ptr) ((ptr) += PPC_INST_LEN(DEREF_PPC_INST_PTR((ptr))))
+#define PPC_INST_PREV(ptr) ((ptr) -= PPC_INST_LEN(DEREF_PPC_INST_PTR((ptr))))
+
+#define PPC_INST_EQ(x, y) \
+({ \
+ long pic_ret = 0; \
+ pic_ret = (PPC_INST_PREFIX(x) == PPC_INST_PREFIX(y)); \
+ if (pic_ret) { \
+ if (PPC_INST_IS_PREFIXED(x) && PPC_INST_IS_PREFIXED(y)) { \
+ pic_ret = (PPC_INST_SUFFIX(x) == PPC_INST_SUFFIX(y)); \
+ } else { \
+ pic_ret = 0; \
+ } \
+ } \
+ pic_ret; \
+})
+
+#else /* !__powerpc64__ */
+
+/* 32 bit Instruction */
+
+typedef unsigned int ppc_inst;
+
+#define PPC_INST_IS_PREFIXED(inst) (0)
+#define PPC_INST_LEN(inst) (4)
+
+#define PPC_INST_NEW_WORD(x) (x)
+#define PPC_INST_NEW_WORD_PAD(x) (x)
+#define PPC_INST_NEW_PREFIXED(x, y) (x)
+
+#define PPC_INST_WORD(x) (x)
+#define PPC_INST_PREFIX(x) (x)
+#define PPC_INST_SUFFIX(x) (0)
+#define PPC_INST_EMPTY(x) (PPC_INST_WORD(x) == 0)
+
+#define DEREF_PPC_INST_PTR(ptr) (*ptr)
+
+#define PPC_INST_NEXT(ptr) ((ptr) += 4)
+#define PPC_INST_PREV(ptr) ((ptr) -= 4)
+
+#define PPC_INST_EQ(x, y) ((x) == (y))
+
+#endif /* __powerpc64__ */
+
+
+#endif /* _ASM_INST_H */
>
> Thanks,
> Nick
^ permalink raw reply related
* Re: [PATCH] macintosh: therm_windtunnel: fix regression when instantiating devices
From: Michael Ellerman @ 2020-02-27 1:23 UTC (permalink / raw)
To: Wolfram Sang, linuxppc-dev
Cc: Mathieu Malaterre, Erhard Furtner, debian-powerpc, linux-i2c,
Wolfram Sang
In-Reply-To: <20200225141229.5424-1-wsa@the-dreams.de>
Wolfram Sang <wsa@the-dreams.de> writes:
> Removing attach_adapter from this driver caused a regression for at
> least some machines. Those machines had the sensors described in their
> DT, too, so they didn't need manual creation of the sensor devices. The
> old code worked, though, because manual creation came first. Creation of
> DT devices then failed later and caused error logs, but the sensors
> worked nonetheless because of the manually created devices.
>
> When removing attach_adaper, manual creation now comes later and loses
> the race. The sensor devices were already registered via DT, yet with
> another binding, so the driver could not be bound to it.
>
> This fix refactors the code to remove the race and only manually creates
> devices if there are no DT nodes present. Also, the DT binding is updated
> to match both, the DT and manually created devices. Because we don't
> know which device creation will be used at runtime, the code to start
> the kthread is moved to do_probe() which will be called by both methods.
>
> Fixes: 3e7bed52719d ("macintosh: therm_windtunnel: drop using attach_adapter")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=201723
> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
> Tested-by: Erhard Furtner <erhard_f@mailbox.org>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> ---
>
> I suggest this stable-tag: # v4.19+
Looks right to me.
> Adding the Debian-PPC List to reach further people maybe willing to
> test.
>
> This patch does not depend on "[PATCH RESEND] macintosh: convert to
> i2c_new_scanned_device". In fact, this one here should go in first as
> 5.6 material. I will rebase and resend the i2c_new_scanned_device()
> conversion on top of this regression fix.
>
> I can also take this via I2C if easier.
I think that would be best, it's more I2C related than powerpc arch
stuff that I could review.
I don't have a machine setup to test this easily, but Erhard has been
doing a good job of testing things so I'm happy for you to take it with
his Tested-by.
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox