* Re: [PATCH] vsprintf: Do not break early boot with probing addresses
From: Sergey Senozhatsky @ 2019-05-10 5:07 UTC (permalink / raw)
To: Linus Torvalds
Cc: Petr Mladek, linux-arch, Sergey Senozhatsky, Heiko Carstens,
linux-s390, Rasmus Villemoes, linux-kernel, Steven Rostedt,
Michal Hocko, Sergey Senozhatsky, Stephen Rothwell,
Andy Shevchenko, linuxppc-dev, Martin Schwidefsky,
Tobin C . Harding
In-Reply-To: <CAHk-=wiP+hwSqEW0dM6AYNWUR7jXDkeueq69et6ahfUgV7hC3w@mail.gmail.com>
On (05/09/19 21:47), Linus Torvalds wrote:
> [ Sorry about html and mobile crud, I'm not at the computer right now ]
> How about we just undo the whole misguided probe_kernel_address() thing?
But the problem will remain - %pS/%pF on PPC (and some other arch-s)
do dereference_function_descriptor(), which calls probe_kernel_address().
So if probe_kernel_address() starts to dump_stack(), then we are heading
towards stack overflow. Unless I'm totally missing something.
-ss
^ permalink raw reply
* Re: [PATCH] vsprintf: Do not break early boot with probing addresses
From: Sergey Senozhatsky @ 2019-05-10 4:32 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-arch, Sergey Senozhatsky, Heiko Carstens, linux-s390,
linuxppc-dev, Rasmus Villemoes, linux-kernel, Steven Rostedt,
Michal Hocko, Sergey Senozhatsky, Stephen Rothwell,
Andy Shevchenko, Linus Torvalds, Martin Schwidefsky,
Tobin C . Harding
In-Reply-To: <20190509121923.8339-1-pmladek@suse.com>
On (05/09/19 14:19), Petr Mladek wrote:
> 1. Report on Power:
>
> Kernel crashes very early during boot with with CONFIG_PPC_KUAP and
> CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG
>
> The problem is the combination of some new code called via printk(),
> check_pointer() which calls probe_kernel_read(). That then calls
> allow_user_access() (PPC_KUAP) and that uses mmu_has_feature() too early
> (before we've patched features). With the JUMP_LABEL debug enabled that
> causes us to call printk() & dump_stack() and we end up recursing and
> overflowing the stack.
Hmm... hmm... PPC does an .opd-based symbol dereference, which
eventually probe_kernel_read()-s. So early printk(%pS) will do
printk(%pS)
dereference_function_descriptor()
probe_kernel_address()
dump_stack()
printk(%pS)
dereference_function_descriptor()
probe_kernel_address()
dump_stack()
printk(%pS)
...
I'd say... that it's not vsprintf that we want to fix, it's
the idea that probe_kernel_address() can dump_stack() on any
platform. On some archs probe_kernel_address()->dump_stack()
is going nowhere:
dump_stack() does probe_kernel_address(), which calls dump_stack(),
which calls printk(%pS)->probe_kernel_address() again and again,
and again.
-ss
^ permalink raw reply
* Re: [PATCH v10 11/12] ima: Define ima-modsig template
From: Mimi Zohar @ 2019-05-09 23:01 UTC (permalink / raw)
To: Thiago Jung Bauermann, linux-integrity
Cc: Herbert Xu, linux-doc, Dmitry Kasatkin, David S. Miller,
Jonathan Corbet, linux-kernel, James Morris, David Howells,
AKASHI, Takahiro, linux-security-module, keyrings, linux-crypto,
Jessica Yu, linuxppc-dev, David Woodhouse, Serge E. Hallyn
In-Reply-To: <20190418035120.2354-12-bauerman@linux.ibm.com>
On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
> Define new "d-modsig" template field which holds the digest that is
> expected to match the one contained in the modsig, and also new "modsig"
> template field which holds the appended file signature.
>
> Add a new "ima-modsig" defined template descriptor with the new fields as
> well as the ones from the "ima-sig" descriptor.
>
> Change ima_store_measurement() to accept a struct modsig * argument so that
> it can be passed along to the templates via struct ima_event_data.
>
> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Thanks, Roberto. Just some thoughts inline below.
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
<snip>
> +/*
> + * Validating the appended signature included in the measurement list requires
> + * the file hash calculated without the appended signature (i.e., the 'd-modsig'
> + * field). Therefore, notify the user if they have the 'modsig' field but not
> + * the 'd-modsig' field in the template.
> + */
> +static void check_current_template_modsig(void)
> +{
> +#define MSG "template with 'modsig' field also needs 'd-modsig' field\n"
> + struct ima_template_desc *template;
> + bool has_modsig, has_dmodsig;
> + static bool checked;
> + int i;
> +
> + /* We only need to notify the user once. */
> + if (checked)
> + return;
> +
> + has_modsig = has_dmodsig = false;
> + template = ima_template_desc_current();
> + for (i = 0; i < template->num_fields; i++) {
> + if (!strcmp(template->fields[i]->field_id, "modsig"))
> + has_modsig = true;
> + else if (!strcmp(template->fields[i]->field_id, "d-modsig"))
> + has_dmodsig = true;
> + }
> +
> + if (has_modsig && !has_dmodsig)
> + pr_notice(MSG);
> +
> + checked = true;
> +#undef MSG
> +}
> +
There was some recent discussion about supporting per IMA policy rule
template formats. This feature will allow just the kexec kernel image
to require ima-modsig. When per policy rule template formats support
is upstreamed, this function will need to be updated.
<snip>
>
> @@ -389,3 +425,25 @@ int ima_eventsig_init(struct ima_event_data *event_data,
> return ima_write_template_field_data(xattr_value, event_data->xattr_len,
> DATA_FMT_HEX, field_data);
> }
> +
> +int ima_eventmodsig_init(struct ima_event_data *event_data,
> + struct ima_field_data *field_data)
> +{
> + const void *data;
> + u32 data_len;
> + int rc;
> +
> + if (!event_data->modsig)
> + return 0;
> +
> + /*
> + * The xattr_value for IMA_MODSIG is a runtime structure containing
> + * pointers. Get its raw data instead.
> + */
"xattr_value"? The comment needs some clarification.
Mimi
> + rc = ima_modsig_serialize(event_data->modsig, &data, &data_len);
> + if (rc)
> + return rc;
> +
> + return ima_write_template_field_data(data, data_len,
> + DATA_FMT_HEX, field_data);
> +}
^ permalink raw reply
* Re: [PATCH v10 09/12] ima: Implement support for module-style appended signatures
From: Mimi Zohar @ 2019-05-09 23:01 UTC (permalink / raw)
To: Thiago Jung Bauermann, linux-integrity
Cc: Herbert Xu, linux-doc, Dmitry Kasatkin, David S. Miller,
Jonathan Corbet, linux-kernel, James Morris, David Howells,
AKASHI, Takahiro, linux-security-module, keyrings, linux-crypto,
Jessica Yu, linuxppc-dev, David Woodhouse, Serge E. Hallyn
In-Reply-To: <20190418035120.2354-10-bauerman@linux.ibm.com>
Hi Thiago,
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index fca7a3f23321..a7a20a8c15c1 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -1144,6 +1144,12 @@ void ima_delete_rules(void)
> }
> }
>
> +#define __ima_hook_stringify(str) (#str),
> +
> +const char *const func_tokens[] = {
> + __ima_hooks(__ima_hook_stringify)
> +};
> +
> #ifdef CONFIG_IMA_READ_POLICY
> enum {
> mask_exec = 0, mask_write, mask_read, mask_append
> @@ -1156,12 +1162,6 @@ static const char *const mask_tokens[] = {
> "MAY_APPEND"
> };
>
> -#define __ima_hook_stringify(str) (#str),
> -
> -static const char *const func_tokens[] = {
> - __ima_hooks(__ima_hook_stringify)
> -};
> -
> void *ima_policy_start(struct seq_file *m, loff_t *pos)
> {
> loff_t l = *pos;
Is moving this something left over from previous versions or there is
a need for this change?
Other than this, the patch looks good.
Mimi
^ permalink raw reply
* Re: [PATCH v2 4/8] mm/memory_hotplug: Create memory block devices after arch_add_memory()
From: David Hildenbrand @ 2019-05-09 22:18 UTC (permalink / raw)
To: Wei Yang
Cc: linux-s390, Michal Hocko, linux-ia64, Pavel Tatashin, linux-sh,
mike.travis@hpe.com, Greg Kroah-Hartman, Rafael J. Wysocki,
Mathieu Malaterre, linux-kernel, Ingo Molnar, linux-mm,
Andrew Banman, Qian Cai, Arun KS, akpm, linuxppc-dev,
Dan Williams, Oscar Salvador
In-Reply-To: <20190509215034.jl2qejw3pzqtbu5d@master>
> Looks good to me.
>
>>
>> (I would actually even prefer "memory_block_devices", because memory
>> blocks have different meanins)
>>
>
> Agree with you, this comes to my mind sometime ago :-)
We have memblocks, memory_blocks ... I guess memory_block_device is
unique :)
>
>>>
>>> [...]
>>>
>>>> /*
>>>> @@ -1106,6 +1100,13 @@ int __ref add_memory_resource(int nid, struct resource *res)
>>>> if (ret < 0)
>>>> goto error;
>>>>
>>>> + /* create memory block devices after memory was added */
>>>> + ret = hotplug_memory_register(start, size);
>>>> + if (ret) {
>>>> + arch_remove_memory(nid, start, size, NULL);
>>>
>>> Functionally, it works I think.
>>>
>>> But arch_remove_memory() would remove pages from zone. At this point, we just
>>> allocate section/mmap for pages, the zones are empty and pages are not
>>> connected to zone.
>>>
>>> Function zone = page_zone(page); always gets zone #0, since pages->flags is 0
>>> at this point. This is not exact.
>>>
>>> Would we add some comment to mention this? Or we need to clean up
>>> arch_remove_memory() to take out __remove_zone()?
>>
>> That is precisely what is on my list next (see cover letter).This is
>> already broken when memory that was never onlined is removed again.
>> So I am planning to fix that independently.
>>
>
> Sounds great :-)
Especially, I suspect a lot of bugs in the area of
1. Remove memory that has never been onlined
2. Remove memory that has been onlined/offlined a couple of times
3. Remove memory that has been onlined to different zones.
Will see when refactoring if my intuition is right :)
>
> Hope you would cc me in the following series.
Sure! Thanks!
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v2 4/8] mm/memory_hotplug: Create memory block devices after arch_add_memory()
From: Wei Yang @ 2019-05-09 21:50 UTC (permalink / raw)
To: David Hildenbrand
Cc: Ingo Molnar, linux-s390, Michal Hocko, linux-ia64, Pavel Tatashin,
linux-sh, mike.travis@hpe.com, Greg Kroah-Hartman,
Rafael J. Wysocki, Mathieu Malaterre, linux-kernel, Wei Yang,
linux-mm, Andrew Banman, Qian Cai, Arun KS, akpm, linuxppc-dev,
Dan Williams, Oscar Salvador
In-Reply-To: <28071389-372c-14eb-1209-02464726b4f0@redhat.com>
On Thu, May 09, 2019 at 04:58:56PM +0200, David Hildenbrand wrote:
>On 09.05.19 16:31, Wei Yang wrote:
>> On Tue, May 07, 2019 at 08:38:00PM +0200, David Hildenbrand wrote:
>>> Only memory to be added to the buddy and to be onlined/offlined by
>>> user space using memory block devices needs (and should have!) memory
>>> block devices.
>>>
>>> Factor out creation of memory block devices Create all devices after
>>> arch_add_memory() succeeded. We can later drop the want_memblock parameter,
>>> because it is now effectively stale.
>>>
>>> Only after memory block devices have been added, memory can be onlined
>>> by user space. This implies, that memory is not visible to user space at
>>> all before arch_add_memory() succeeded.
>>>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> Cc: "mike.travis@hpe.com" <mike.travis@hpe.com>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Ingo Molnar <mingo@kernel.org>
>>> Cc: Andrew Banman <andrew.banman@hpe.com>
>>> Cc: Oscar Salvador <osalvador@suse.de>
>>> Cc: Michal Hocko <mhocko@suse.com>
>>> Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
>>> Cc: Qian Cai <cai@lca.pw>
>>> Cc: Wei Yang <richard.weiyang@gmail.com>
>>> Cc: Arun KS <arunks@codeaurora.org>
>>> Cc: Mathieu Malaterre <malat@debian.org>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> ---
>>> drivers/base/memory.c | 70 ++++++++++++++++++++++++++----------------
>>> include/linux/memory.h | 2 +-
>>> mm/memory_hotplug.c | 15 ++++-----
>>> 3 files changed, 53 insertions(+), 34 deletions(-)
>>>
>>> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
>>> index 6e0cb4fda179..862c202a18ca 100644
>>> --- a/drivers/base/memory.c
>>> +++ b/drivers/base/memory.c
>>> @@ -701,44 +701,62 @@ static int add_memory_block(int base_section_nr)
>>> return 0;
>>> }
>>>
>>> +static void unregister_memory(struct memory_block *memory)
>>> +{
>>> + BUG_ON(memory->dev.bus != &memory_subsys);
>>> +
>>> + /* drop the ref. we got via find_memory_block() */
>>> + put_device(&memory->dev);
>>> + device_unregister(&memory->dev);
>>> +}
>>> +
>>> /*
>>> - * need an interface for the VM to add new memory regions,
>>> - * but without onlining it.
>>> + * Create memory block devices for the given memory area. Start and size
>>> + * have to be aligned to memory block granularity. Memory block devices
>>> + * will be initialized as offline.
>>> */
>>> -int hotplug_memory_register(int nid, struct mem_section *section)
>>> +int hotplug_memory_register(unsigned long start, unsigned long size)
>>
>> One trivial suggestion about the function name.
>>
>> For memory_block device, sometimes we use the full name
>>
>> find_memory_block
>> init_memory_block
>> add_memory_block
>>
>> But sometimes we use *nick* name
>>
>> hotplug_memory_register
>> register_memory
>> unregister_memory
>>
>> This is a little bit confusion.
>>
>> Can we use one name convention here?
>
>We can just go for
>
>crate_memory_blocks() and free_memory_blocks(). Or do
>you have better suggestions?
s/crate/create/
Looks good to me.
>
>(I would actually even prefer "memory_block_devices", because memory
>blocks have different meanins)
>
Agree with you, this comes to my mind sometime ago :-)
>>
>> [...]
>>
>>> /*
>>> @@ -1106,6 +1100,13 @@ int __ref add_memory_resource(int nid, struct resource *res)
>>> if (ret < 0)
>>> goto error;
>>>
>>> + /* create memory block devices after memory was added */
>>> + ret = hotplug_memory_register(start, size);
>>> + if (ret) {
>>> + arch_remove_memory(nid, start, size, NULL);
>>
>> Functionally, it works I think.
>>
>> But arch_remove_memory() would remove pages from zone. At this point, we just
>> allocate section/mmap for pages, the zones are empty and pages are not
>> connected to zone.
>>
>> Function zone = page_zone(page); always gets zone #0, since pages->flags is 0
>> at this point. This is not exact.
>>
>> Would we add some comment to mention this? Or we need to clean up
>> arch_remove_memory() to take out __remove_zone()?
>
>That is precisely what is on my list next (see cover letter).This is
>already broken when memory that was never onlined is removed again.
>So I am planning to fix that independently.
>
Sounds great :-)
Hope you would cc me in the following series.
>
>--
>
>Thanks,
>
>David / dhildenb
--
Wei Yang
Help you, Help me
^ permalink raw reply
* Re: [PATCH] EDAC, mpc85xx: Prevent building as a module
From: Borislav Petkov @ 2019-05-09 18:02 UTC (permalink / raw)
To: Michael Ellerman
Cc: Johannes Thumshirn, linux-kernel, linuxppc-dev, james.morse,
mchehab, linux-edac
In-Reply-To: <20190509145534.GD17053@zn.tnic>
On Thu, May 09, 2019 at 04:55:34PM +0200, Borislav Petkov wrote:
> On Fri, May 10, 2019 at 12:52:05AM +1000, Michael Ellerman wrote:
> > Thanks. It would be nice if you could send it as a fix for 5.2, it's the
> > last thing blocking one of my allmodconfig builds. But if you don't
> > think it qualifies as a fix that's fine too, it can wait.
>
> Sure, no problem. Will do a pull request later.
Hmm, so looking at this more, I was able to produce this config with my
ancient cross-compiler:
CONFIG_EDAC_SUPPORT=y
CONFIG_EDAC=m
CONFIG_EDAC_LEGACY_SYSFS=y
CONFIG_EDAC_MPC85XX=y
Now, mpc85xx_edac is built-in and edac_core.ko is a module
(CONFIG_EDAC=m) and that should not work - i.e., builtin code calling
module functions. But my cross-compiler is happily building this without
complaint. Or maybe I'm missing something.
In any case, I *think* the proper fix should be to do:
config EDAC_MPC85XX
bool "Freescale MPC83xx / MPC85xx"
depends on FSL_SOC && EDAC=y
so that you can't even produce the above invalid .config snippet.
Hmmm?
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* RE: [PATCH] vsprintf: Do not break early boot with probing addresses
From: David Laight @ 2019-05-09 13:46 UTC (permalink / raw)
To: 'Michal Suchánek', Petr Mladek
Cc: linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
Sergey Senozhatsky, Rasmus Villemoes,
linuxppc-dev@lists.ozlabs.org, Heiko Carstens,
linux-kernel@vger.kernel.org, Steven Rostedt, Michal Hocko,
Sergey Senozhatsky, Stephen Rothwell, Andy Shevchenko,
Linus Torvalds, Martin Schwidefsky, Tobin C . Harding
In-Reply-To: <20190509153829.06319d0c@kitsune.suse.cz>
From: Michal Suchánek
> Sent: 09 May 2019 14:38
...
> > The problem is the combination of some new code called via printk(),
> > check_pointer() which calls probe_kernel_read(). That then calls
> > allow_user_access() (PPC_KUAP) and that uses mmu_has_feature() too early
> > (before we've patched features).
>
> There is early_mmu_has_feature for this case. mmu_has_feature does not
> work before patching so parts of kernel that can run before patching
> must use the early_ variant which actually runs code reading the
> feature bitmap to determine the answer.
Does the early_ variant get patched so the it is reasonably
efficient after the 'patching' is done?
Or should there be a third version which gets patched across?
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [next-20190507][powerpc] WARN kernel/cgroup/cgroup.c:6008 with LTP ptrace01 test case
From: Roman Gushchin @ 2019-05-09 16:27 UTC (permalink / raw)
To: Sachin Sant
Cc: Tejun Heo, linux-next@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, Oleg Nesterov
In-Reply-To: <B292CCEE-B9ED-4227-A6F7-ADAD93011F1A@linux.vnet.ibm.com>
On Thu, May 09, 2019 at 11:35:36AM +0530, Sachin Sant wrote:
>
>
> > On 09-May-2019, at 4:53 AM, Roman Gushchin <guro@fb.com> wrote:
> >
> > On Wed, May 08, 2019 at 03:06:30PM +0530, Sachin Sant wrote:
> >> While running LTP tests(specifically ptrace01) following WARNING is observed
> >> on POWER8 LPAR running next-20190507 built using 4K page size.
> >>
> >> [ 3969.979492] msgrcv04 (433) used greatest stack depth: 9328 bytes left
> >> [ 3981.452911] madvise06 (515): drop_caches: 3
> >> [ 4004.575752] WARNING: CPU: 5 PID: 721 at kernel/cgroup/cgroup.c:6008 cgroup_exit+0x2ac/0x2c0
> >> [ 4004.575781] Modules linked in: overlay rpadlpar_io rpaphp iptable_mangle xt_MASQUERADE iptable_nat nf_nat xt_conntrack nf_conntrack nf_defrag_ipv4 ipt_REJECT nf_reject_ipv4 xt_tcpudp tun bridge stp llc kvm iptable_filter pseries_rng rng_core vmx_crypto ip_tables x_tables autofs4 [last unloaded: dummy_del_mod]
> >> [ 4004.575837] CPU: 5 PID: 721 Comm: ptrace01 Tainted: G O 5.1.0-next-20190507-autotest #1
> >> [ 4004.575846] NIP: c000000001b3026c LR: c000000001b30054 CTR: c000000001c9f020
> >> [ 4004.575855] REGS: c000000171fff840 TRAP: 0700 Tainted: G O (5.1.0-next-20190507-autotest)
> >> [ 4004.575863] MSR: 800000010282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE,TM[E]> CR: 44004824 XER: 20000000
> >> [ 4004.575885] CFAR: c000000001b30078 IRQMASK: 1
> >> [ 4004.575885] GPR00: c000000001b30054 c000000171fffad0 c000000003938700 c00000027b02fa18
> >> [ 4004.575885] GPR04: c00000027b02fa00 0000000000000000 c000000003ae8700 00000000001c180a
> >> [ 4004.575885] GPR08: 0000000000000001 0000000000000001 c000000003ae8700 0000000000000001
> >> [ 4004.575885] GPR12: 0000000000004400 c00000001ec7ea80 c000000003a4d670 0000000000000009
> >> [ 4004.575885] GPR16: 0000000000000000 0000000000040100 00000000418004fc 0000000008430000
> >> [ 4004.575885] GPR20: 0000000000000009 0000000000000001 c0000001715e9200 c00000016d8f4d00
> >> [ 4004.575885] GPR24: c000000171fffd90 0000000000000100 c000000168692478 c000000171fffb98
> >> [ 4004.575885] GPR28: c000000168692400 c00000016d8f4d00 c0000000036420d0 c00000027b02fa00
> >> [ 4004.575958] NIP [c000000001b3026c] cgroup_exit+0x2ac/0x2c0
> >> [ 4004.575966] LR [c000000001b30054] cgroup_exit+0x94/0x2c0
> >> [ 4004.575972] Call Trace:
> >> [ 4004.575979] [c000000171fffad0] [c000000001b30054] cgroup_exit+0x94/0x2c0 (unreliable)
> >> [ 4004.575990] [c000000171fffb30] [c0000000019cea98] do_exit+0x878/0x1ae0
> >> [ 4004.575999] [c000000171fffc00] [c0000000019cfe4c] do_group_exit+0xac/0x1d0
> >> [ 4004.576009] [c000000171fffc40] [c0000000019ed00c] get_signal+0x2bc/0x11c0
> >> [ 4004.576019] [c000000171fffd30] [c000000001867b14] do_notify_resume+0x384/0x900
> >> [ 4004.576029] [c000000171fffe20] [c00000000183e844] ret_from_except_lite+0x70/0x74
> >> [ 4004.576037] Instruction dump:
> >> [ 4004.576043] 314a0001 7d40492d 40c2fff4 3d42001b e92a7288 39290001 f92a7288 4bfffe5c
> >> [ 4004.576056] 3d42001b e92a7258 39290001 f92a7258 <0fe00000> 4bfffe0c 4be91e45 60000000
> >> [ 4004.576071] ---[ end trace 82a1a7c19005ebd6 ]—
> >>
> >> The WARN_ONCE was added by following commit
> >> 96b9c592def5 ("cgroup: get rid of cgroup_freezer_frozen_exit()”).
> >>
> >> Reverting the patch helps avoid the warning.
> >
> > Hi Sachin!
> >
> > Thank you for the report!
> >
> > Can you, please, check that the patch at https://lkml.org/lkml/2019/5/8/938 <https://lkml.org/lkml/2019/5/8/938>
> > solves the problem?
> >
> This patch fixes the problem for me.
Thank you for the confirmation!
The patch has just been pulled into the cgroup tree,
and will appear in next soon.
Thanks!
^ permalink raw reply
* Re: Build failure in v4.4.y.queue (ppc:allmodconfig)
From: Guenter Roeck @ 2019-05-09 16:23 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Greg Kroah-Hartman, linuxppc-dev, stable
In-Reply-To: <87zhnvvgwm.fsf@concordia.ellerman.id.au>
On Fri, May 10, 2019 at 12:31:05AM +1000, Michael Ellerman wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > On Wed, May 08, 2019 at 01:26:42PM -0700, Guenter Roeck wrote:
> >> I see multiple instances of:
> >>
> >> arch/powerpc/kernel/exceptions-64s.S:839: Error:
> >> attempt to move .org backwards
> >>
> >> in v4.4.y.queue (v4.4.179-143-gc4db218e9451).
> >>
> >> This is due to commit 9b2d4e06d7f1 ("powerpc/64s: Add support for a store
> >> forwarding barrier at kernel entry/exit"), which is part of a large patch
> >> series and can not easily be reverted.
> >>
> >> Guess I'll stop doing ppc:allmodconfig builds in v4.4.y ?
> >
> > Michael, I thought this patch series was supposed to fix ppc issues, not
> > add to them :)
>
> Well it fixes some, but creates others :}
>
> > Any ideas on what to do here?
>
> Turning off CONFIG_CBE_RAS (obscure IBM Cell Blade RAS features) is
> sufficient to get it building. Is that an option for your build tests
> Guenter?
>
I could turn it off unconditionally, meaning it would affect all other
branches. I would rather stop building ppc:allmodconfig for v4.4.y.
Guenter
^ permalink raw reply
* Re: [PATCH linux-next v10 0/7] ptrace: add PTRACE_GET_SYSCALL_INFO request
From: Oleg Nesterov @ 2019-05-09 16:14 UTC (permalink / raw)
To: Dmitry V. Levin, Andrew Morton
Cc: James E.J. Bottomley, Paul Mackerras, linux-kselftest,
Vincent Chen, Shuah Khan, Helge Deller, Eugene Syromyatnikov,
Elvira Khabirova, James Hogan, strace-devel, Kees Cook,
Greentime Hu, linux-kernel, Andy Lutomirski, linux-parisc,
linux-api, linux-mips, Ralf Baechle, Richard Kuo, Paul Burton,
linux-hexagon, linuxppc-dev
In-Reply-To: <20190415234307.GA9364@altlinux.org>
On 04/16, Dmitry V. Levin wrote:
>
> [Andrew, could you take this patchset into your tree, please?]
Just in case...
I have already acked 6/7.
Other patches look good to me too, just I don't think I can actually review
these non-x86 changes.
Oleg.
^ permalink raw reply
* Re: [PATCH v10 08/12] ima: Factor xattr_verify() out of ima_appraise_measurement()
From: Mimi Zohar @ 2019-05-09 15:53 UTC (permalink / raw)
To: Thiago Jung Bauermann, linux-integrity
Cc: Herbert Xu, linux-doc, Dmitry Kasatkin, David S. Miller,
Jonathan Corbet, linux-kernel, James Morris, David Howells,
AKASHI, Takahiro, linux-security-module, keyrings, linux-crypto,
Jessica Yu, linuxppc-dev, David Woodhouse, Serge E. Hallyn
In-Reply-To: <20190418035120.2354-9-bauerman@linux.ibm.com>
On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
> Verify xattr signature in a separate function so that the logic in
> ima_appraise_measurement() remains clear when it gains the ability to also
> verify an appended module signature.
>
> The code in the switch statement is unchanged except for having to
> dereference the status and cause variables (since they're now pointers),
> and fixing the style of a block comment to appease checkpatch.
>
> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v10 06/12] ima: Use designated initializers for struct ima_event_data
From: Mimi Zohar @ 2019-05-09 15:46 UTC (permalink / raw)
To: Thiago Jung Bauermann, linux-integrity
Cc: Herbert Xu, linux-doc, Dmitry Kasatkin, David S. Miller,
Jonathan Corbet, linux-kernel, James Morris, David Howells,
AKASHI, Takahiro, linux-security-module, keyrings, linux-crypto,
Jessica Yu, linuxppc-dev, David Woodhouse, Serge E. Hallyn
In-Reply-To: <20190418035120.2354-7-bauerman@linux.ibm.com>
On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
> Designated initializers allow specifying only the members of the struct
> that need initialization. Non-mentioned members are initialized to zero.
>
> This makes the code a bit clearer (particularly in ima_add_boot_aggregate()
> and also allows adding a new member to the struct without having to update
> all struct initializations.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
> security/integrity/ima/ima_api.c | 11 +++++++----
> security/integrity/ima/ima_init.c | 4 ++--
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c7505fb122d4..0639d0631f2c 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -133,8 +133,9 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
> {
> struct ima_template_entry *entry;
> struct inode *inode = file_inode(file);
> - struct ima_event_data event_data = {iint, file, filename, NULL, 0,
> - cause};
> + struct ima_event_data event_data = { .iint = iint, .file = file,
> + .filename = filename,
> + .violation = cause };
> int violation = 1;
> int result;
>
> @@ -284,8 +285,10 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
> int result = -ENOMEM;
> struct inode *inode = file_inode(file);
> struct ima_template_entry *entry;
> - struct ima_event_data event_data = {iint, file, filename, xattr_value,
> - xattr_len, NULL};
> + struct ima_event_data event_data = { .iint = iint, .file = file,
> + .filename = filename,
> + .xattr_value = xattr_value,
> + .xattr_len = xattr_len };
> int violation = 0;
>
> if (iint->measured_pcrs & (0x1 << pcr))
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 6c9295449751..ef6c3a26296e 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -49,8 +49,8 @@ static int __init ima_add_boot_aggregate(void)
> const char *audit_cause = "ENOMEM";
> struct ima_template_entry *entry;
> struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
> - struct ima_event_data event_data = {iint, NULL, boot_aggregate_name,
> - NULL, 0, NULL};
> + struct ima_event_data event_data = { .iint = iint,
> + .filename = boot_aggregate_name };
> int result = -ENOMEM;
> int violation = 0;
> struct {
^ permalink raw reply
* Re: [PATCH v10 02/12] PKCS#7: Refactor verify_pkcs7_signature()
From: Mimi Zohar @ 2019-05-09 15:42 UTC (permalink / raw)
To: Thiago Jung Bauermann, linux-integrity
Cc: Herbert Xu, linux-doc, Dmitry Kasatkin, David S. Miller,
Jonathan Corbet, linux-kernel, James Morris, David Howells,
AKASHI, Takahiro, linux-security-module, keyrings, linux-crypto,
Jessica Yu, linuxppc-dev, David Woodhouse, Serge E. Hallyn
In-Reply-To: <20190418035120.2354-3-bauerman@linux.ibm.com>
On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
> IMA will need to verify a PKCS#7 signature which has already been parsed.
> For this reason, factor out the code which does that from
> verify_pkcs7_signature() into a new function which takes a struct
> pkcs7_message instead of a data buffer.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v10 03/12] PKCS#7: Introduce pkcs7_get_digest()
From: Mimi Zohar @ 2019-05-09 15:42 UTC (permalink / raw)
To: Thiago Jung Bauermann, linux-integrity
Cc: Herbert Xu, linux-doc, Dmitry Kasatkin, David S. Miller,
Jonathan Corbet, linux-kernel, James Morris, David Howells,
AKASHI, Takahiro, linux-security-module, keyrings, linux-crypto,
Jessica Yu, linuxppc-dev, David Woodhouse, Serge E. Hallyn
In-Reply-To: <20190418035120.2354-4-bauerman@linux.ibm.com>
On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
> IMA will need to access the digest of the PKCS7 message (as calculated by
> the kernel) before the signature is verified, so introduce
> pkcs7_get_digest() for that purpose.
>
> Also, modify pkcs7_digest() to detect when the digest was already
> calculated so that it doesn't have to do redundant work. Verifying that
> sinfo->sig->digest isn't NULL is sufficient because both places which
> allocate sinfo->sig (pkcs7_parse_message() and pkcs7_note_signed_info())
> use kzalloc() so sig->digest is always initialized to zero.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v10 01/12] MODSIGN: Export module signature definitions
From: Mimi Zohar @ 2019-05-09 15:42 UTC (permalink / raw)
To: Thiago Jung Bauermann, linux-integrity
Cc: Herbert Xu, linux-doc, Dmitry Kasatkin, David S. Miller,
Jonathan Corbet, linux-kernel, James Morris, David Howells,
AKASHI, Takahiro, linux-security-module, keyrings, linux-crypto,
Jessica Yu, linuxppc-dev, David Woodhouse, Serge E. Hallyn
In-Reply-To: <20190418035120.2354-2-bauerman@linux.ibm.com>
On Thu, 2019-04-18 at 00:51 -0300, Thiago Jung Bauermann wrote:
> IMA will use the module_signature format for append signatures, so export
> the relevant definitions and factor out the code which verifies that the
> appended signature trailer is valid.
>
> Also, create a CONFIG_MODULE_SIG_FORMAT option so that IMA can select it
> and be able to use mod_check_sig() without having to depend on either
> CONFIG_MODULE_SIG or CONFIG_MODULES.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Cc: Jessica Yu <jeyu@kernel.org>
Just a couple minor questions/comments below.
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
< snip >
> diff --git a/init/Kconfig b/init/Kconfig
> index 4592bf7997c0..a71019553ee1 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1906,7 +1906,7 @@ config MODULE_SRCVERSION_ALL
> config MODULE_SIG
> bool "Module signature verification"
> depends on MODULES
> - select SYSTEM_DATA_VERIFICATION
> + select MODULE_SIG_FORMAT
> help
> Check modules for valid signatures upon load: the signature
> is simply appended to the module. For more information see
> @@ -2036,6 +2036,10 @@ config TRIM_UNUSED_KSYMS
>
> endif # MODULES
>
> +config MODULE_SIG_FORMAT
> + def_bool n
> + select SYSTEM_DATA_VERIFICATION
Normally Kconfigs, in the same file, are defined before they are used.
I'm not sure if that is required or just a convention.
> config MODULES_TREE_LOOKUP
> def_bool y
> depends on PERF_EVENTS || TRACING
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 6c57e78817da..d2f2488f80ab 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -57,6 +57,7 @@ endif
> obj-$(CONFIG_UID16) += uid16.o
> obj-$(CONFIG_MODULES) += module.o
> obj-$(CONFIG_MODULE_SIG) += module_signing.o
> +obj-$(CONFIG_MODULE_SIG_FORMAT) += module_signature.o
> obj-$(CONFIG_KALLSYMS) += kallsyms.o
> obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
> obj-$(CONFIG_CRASH_CORE) += crash_core.o
> diff --git a/kernel/module.c b/kernel/module.c
> index 985caa467aef..326ddeb364dd 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -19,6 +19,7 @@
> #include <linux/export.h>
> #include <linux/extable.h>
> #include <linux/moduleloader.h>
> +#include <linux/module_signature.h>
> #include <linux/trace_events.h>
> #include <linux/init.h>
> #include <linux/kallsyms.h>
> diff --git a/kernel/module_signature.c b/kernel/module_signature.c
> new file mode 100644
> index 000000000000..6d5e59f27f55
> --- /dev/null
> +++ b/kernel/module_signature.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Module signature checker
> + *
> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/printk.h>
> +#include <linux/module_signature.h>
> +#include <asm/byteorder.h>
> +
> +/**
> + * mod_check_sig - check that the given signature is sane
> + *
> + * @ms: Signature to check.
> + * @file_len: Size of the file to which @ms is appended.
"name" is missing.
Mimi
> + */
^ permalink raw reply
* Re: [PATCH] powerpc/64s: Use early_mmu_has_feature() in set_kuap()
From: Michael Ellerman @ 2019-05-09 15:34 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20190508123047.10217-1-mpe@ellerman.id.au>
On Wed, 2019-05-08 at 12:30:47 UTC, Michael Ellerman wrote:
> When implementing the KUAP support on Radix we fixed one case where
> mmu_has_feature() was being called too early in boot via
> __put_user_size().
>
> However since then some new code in linux-next has created a new path
> via which we can end up calling mmu_has_feature() too early.
>
> On P9 this leads to crashes early in boot if we have both PPC_KUAP and
> CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG enabled. Our early boot code
> calls printk() which calls probe_kernel_read(), that does a
> __copy_from_user_inatomic() which calls into set_kuap() and that uses
> mmu_has_feature().
>
> At that point in boot we haven't patched MMU features yet so the debug
> code in mmu_has_feature() complains, and calls printk(). At that point
> we recurse, eg:
>
> ...
> dump_stack+0xdc
> probe_kernel_read+0x1a4
> check_pointer+0x58
> ...
> printk+0x40
> dump_stack_print_info+0xbc
> dump_stack+0x8
> probe_kernel_read+0x1a4
> probe_kernel_read+0x19c
> check_pointer+0x58
> ...
> printk+0x40
> cpufeatures_process_feature+0xc8
> scan_cpufeatures_subnodes+0x380
> of_scan_flat_dt_subnodes+0xb4
> dt_cpu_ftrs_scan_callback+0x158
> of_scan_flat_dt+0xf0
> dt_cpu_ftrs_scan+0x3c
> early_init_devtree+0x360
> early_setup+0x9c
>
> And so on for infinity, symptom is a dead system.
>
> Even more fun is what happens when using the hash MMU (ie. p8 or p9
> with Radix disabled), and when we don't have
> CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG enabled. With the debug disabled
> we don't check if static keys have been initialised, we just rely on
> the jump label. But the jump label defaults to true so we just whack
> the AMR even though Radix is not enabled.
>
> Clearing the AMR is fine, but after we've done the user copy we write
> (0b11 << 62) into AMR. When using hash that makes all pages with key
> zero no longer readable or writable. All kernel pages implicitly have
> key zero, and so all of a sudden the kernel can't read or write any of
> its memory. Again dead system.
>
> In the medium term we have several options for fixing this.
> probe_kernel_read() doesn't need to touch AMR at all, it's not doing a
> user access after all, but it uses __copy_from_user_inatomic() just
> because it's easy, we could fix that.
>
> It would also be safe to default to not writing to the AMR during
> early boot, until we've detected features. But it's not clear that
> flipping all the MMU features to static_key_false won't introduce
> other bugs.
>
> But for now just switch to early_mmu_has_feature() in set_kuap(), that
> avoids all the problems with jump labels. It adds the overhead of a
> global lookup and test, but that's probably trivial compared to the
> writes to the AMR anyway.
>
> Fixes: 890274c2dc4c ("powerpc/64s: Implement KUAP for Radix MMU")
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Reviewed-by: Russell Currey <ruscur@russell.cc>
Applied to powerpc next.
https://git.kernel.org/powerpc/c/8150a153c013aa2dd1ffae43370b89ac
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/book3s/64: check for NULL pointer in pgd_alloc()
From: Michael Ellerman @ 2019-05-09 15:34 UTC (permalink / raw)
To: Rick Lindsley, Michael Ellerman, linuxppc-dev
In-Reply-To: <5ae0401c-a84a-1e59-5d9d-70659f5a85de@linux.vnet.ibm.com>
On Mon, 2019-05-06 at 00:20:43 UTC, Rick Lindsley wrote:
> When the memset code was added to pgd_alloc(), it failed to consider that kmem_cache_alloc() can return NULL. It's uncommon, but not impossible under heavy memory contention.
>
> Signed-off-by: Rick Lindsley <ricklind@vnet.linux.ibm.com>
> Fixes: cf266dbcd2a7 ("Zero PGD pages on allocation")
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/f39356261c265a0689d7ee568132d516
cheers
^ permalink raw reply
* Re: [PATCH v8 05/20] KVM: PPC: Book3S HV: Remove pmd_is_leaf()
From: Steven Price @ 2019-05-09 15:03 UTC (permalink / raw)
To: Paul Mackerras
Cc: Mark Rutland, Peter Zijlstra, Catalin Marinas, Dave Hansen,
Will Deacon, linux-mm, H. Peter Anvin, Liang, Kan, x86,
Ingo Molnar, Arnd Bergmann, kvm-ppc, Jérôme Glisse,
Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
linux-arm-kernel, Ard Biesheuvel, linux-kernel, James Morse,
Andrew Morton, linuxppc-dev
In-Reply-To: <20190429020555.GB11154@blackberry>
On 29/04/2019 03:05, Paul Mackerras wrote:
> On Wed, Apr 03, 2019 at 03:16:12PM +0100, Steven Price wrote:
>> Since pmd_large() is now always available, pmd_is_leaf() is redundant.
>> Replace all uses with calls to pmd_large().
>
> NAK. I don't want to do this, because pmd_is_leaf() is purely about
> the guest page tables (the "partition-scoped" radix tree which
> specifies the guest physical to host physical translation), not about
> anything to do with the Linux process page tables. The guest page
> tables have the same format as the Linux process page tables, but they
> are managed separately.
Fair enough, I'll drop this patch in the next posting.
> If it makes things clearer, I could rename it to "guest_pmd_is_leaf()"
> or something similar.
I'll leave that decision up to you - it might prevent similar confusion
in the future.
Steve
^ permalink raw reply
* Re: [PATCH v2 4/8] mm/memory_hotplug: Create memory block devices after arch_add_memory()
From: David Hildenbrand @ 2019-05-09 14:58 UTC (permalink / raw)
To: Wei Yang
Cc: linux-s390, Michal Hocko, linux-ia64, Pavel Tatashin, linux-sh,
mike.travis@hpe.com, Greg Kroah-Hartman, Rafael J. Wysocki,
Mathieu Malaterre, linux-kernel, Ingo Molnar, linux-mm,
Andrew Banman, Qian Cai, Arun KS, akpm, linuxppc-dev,
Dan Williams, Oscar Salvador
In-Reply-To: <20190509143151.zexjmwu3ikkmye7i@master>
On 09.05.19 16:31, Wei Yang wrote:
> On Tue, May 07, 2019 at 08:38:00PM +0200, David Hildenbrand wrote:
>> Only memory to be added to the buddy and to be onlined/offlined by
>> user space using memory block devices needs (and should have!) memory
>> block devices.
>>
>> Factor out creation of memory block devices Create all devices after
>> arch_add_memory() succeeded. We can later drop the want_memblock parameter,
>> because it is now effectively stale.
>>
>> Only after memory block devices have been added, memory can be onlined
>> by user space. This implies, that memory is not visible to user space at
>> all before arch_add_memory() succeeded.
>>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: "mike.travis@hpe.com" <mike.travis@hpe.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Andrew Banman <andrew.banman@hpe.com>
>> Cc: Oscar Salvador <osalvador@suse.de>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
>> Cc: Qian Cai <cai@lca.pw>
>> Cc: Wei Yang <richard.weiyang@gmail.com>
>> Cc: Arun KS <arunks@codeaurora.org>
>> Cc: Mathieu Malaterre <malat@debian.org>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> drivers/base/memory.c | 70 ++++++++++++++++++++++++++----------------
>> include/linux/memory.h | 2 +-
>> mm/memory_hotplug.c | 15 ++++-----
>> 3 files changed, 53 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
>> index 6e0cb4fda179..862c202a18ca 100644
>> --- a/drivers/base/memory.c
>> +++ b/drivers/base/memory.c
>> @@ -701,44 +701,62 @@ static int add_memory_block(int base_section_nr)
>> return 0;
>> }
>>
>> +static void unregister_memory(struct memory_block *memory)
>> +{
>> + BUG_ON(memory->dev.bus != &memory_subsys);
>> +
>> + /* drop the ref. we got via find_memory_block() */
>> + put_device(&memory->dev);
>> + device_unregister(&memory->dev);
>> +}
>> +
>> /*
>> - * need an interface for the VM to add new memory regions,
>> - * but without onlining it.
>> + * Create memory block devices for the given memory area. Start and size
>> + * have to be aligned to memory block granularity. Memory block devices
>> + * will be initialized as offline.
>> */
>> -int hotplug_memory_register(int nid, struct mem_section *section)
>> +int hotplug_memory_register(unsigned long start, unsigned long size)
>
> One trivial suggestion about the function name.
>
> For memory_block device, sometimes we use the full name
>
> find_memory_block
> init_memory_block
> add_memory_block
>
> But sometimes we use *nick* name
>
> hotplug_memory_register
> register_memory
> unregister_memory
>
> This is a little bit confusion.
>
> Can we use one name convention here?
We can just go for
crate_memory_blocks() and free_memory_blocks(). Or do
you have better suggestions?
(I would actually even prefer "memory_block_devices", because memory
blocks have different meanins)
>
> [...]
>
>> /*
>> @@ -1106,6 +1100,13 @@ int __ref add_memory_resource(int nid, struct resource *res)
>> if (ret < 0)
>> goto error;
>>
>> + /* create memory block devices after memory was added */
>> + ret = hotplug_memory_register(start, size);
>> + if (ret) {
>> + arch_remove_memory(nid, start, size, NULL);
>
> Functionally, it works I think.
>
> But arch_remove_memory() would remove pages from zone. At this point, we just
> allocate section/mmap for pages, the zones are empty and pages are not
> connected to zone.
>
> Function zone = page_zone(page); always gets zone #0, since pages->flags is 0
> at this point. This is not exact.
>
> Would we add some comment to mention this? Or we need to clean up
> arch_remove_memory() to take out __remove_zone()?
That is precisely what is on my list next (see cover letter).This is
already broken when memory that was never onlined is removed again.
So I am planning to fix that independently.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH] EDAC, mpc85xx: Prevent building as a module
From: Borislav Petkov @ 2019-05-09 14:55 UTC (permalink / raw)
To: Michael Ellerman
Cc: Johannes Thumshirn, linux-kernel, linuxppc-dev, james.morse,
mchehab, linux-edac
In-Reply-To: <87o94bvfxm.fsf@concordia.ellerman.id.au>
On Fri, May 10, 2019 at 12:52:05AM +1000, Michael Ellerman wrote:
> Thanks. It would be nice if you could send it as a fix for 5.2, it's the
> last thing blocking one of my allmodconfig builds. But if you don't
> think it qualifies as a fix that's fine too, it can wait.
Sure, no problem. Will do a pull request later.
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* Re: [PATCH] EDAC, mpc85xx: Prevent building as a module
From: Michael Ellerman @ 2019-05-09 14:52 UTC (permalink / raw)
To: Borislav Petkov, Johannes Thumshirn
Cc: linuxppc-dev, mchehab, james.morse, linux-kernel, linux-edac
In-Reply-To: <20190508101238.GB19015@zn.tnic>
Borislav Petkov <bp@alien8.de> writes:
> On Mon, May 06, 2019 at 08:50:45AM +0200, Johannes Thumshirn wrote:
>> Acked-by: Johannes Thumshirn <jth@kernel.org>
>
> Queued, thanks.
Thanks. It would be nice if you could send it as a fix for 5.2, it's the
last thing blocking one of my allmodconfig builds. But if you don't
think it qualifies as a fix that's fine too, it can wait.
cheers
^ permalink raw reply
* Re: [PATCH] crypto: caam/jr - Remove extra memory barrier during job ring dequeue
From: Michael Ellerman @ 2019-05-09 14:48 UTC (permalink / raw)
To: Herbert Xu, Horia Geanta
Cc: Vakul Garg, Aymen Sghaier, linuxppc-dev@lists.ozlabs.org,
linux-crypto@vger.kernel.org, davem@davemloft.net
In-Reply-To: <20190509052352.nje7a4niuc2n6c57@gondor.apana.org.au>
Herbert Xu <herbert@gondor.apana.org.au> writes:
> On Thu, May 02, 2019 at 11:08:55AM +0000, Horia Geanta wrote:
>> >> +
>> >> static inline void wr_reg32(void __iomem *reg, u32 data)
>> >> {
>> >> if (caam_little_end)
>> >
>> > This crashes on my p5020ds. Did you test on powerpc?
>> >
>> > # first bad commit: [bbfcac5ff5f26aafa51935a62eb86b6eacfe8a49] crypto: caam/jr - Remove extra memory barrier during job ring dequeue
>>
>> Thanks for the report Michael.
>>
>> Any hint what would be the proper approach here - to have relaxed I/O accessors
>> that would work both for ARM and PPC, and avoid ifdeffery etc.?
>
> Since no fix has been found I'm reverting the commit in question.
Thanks.
Sorry I haven't had time to look into it with everything else going on
during the merge window.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: Fix compile issue with force DAWR
From: Michael Ellerman @ 2019-05-09 14:46 UTC (permalink / raw)
To: Christophe Leroy, Michael Neuling; +Cc: linuxppc-dev
In-Reply-To: <0e2e24dc-4533-4439-3dc1-97ec64fbf49b@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 08/05/2019 à 03:30, Michael Neuling a écrit :
>> If you compile with KVM but without CONFIG_HAVE_HW_BREAKPOINT you fail
>> at linking with:
>> arch/powerpc/kvm/book3s_hv_rmhandlers.o:(.text+0x708): undefined reference to `dawr_force_enable'
>>
>> This was caused by this recent patch:
>> commit c1fe190c06723322f2dfac31d3b982c581e434ef
>> Author: Michael Neuling <mikey@neuling.org>
>> powerpc: Add force enable of DAWR on P9 option
>
> I think you should use the standard commit format, checkpatch will tell you.
>
>>
>> This builds dawr_force_enable in always via a new file.
>
> Do we really need a new file just for that ?
I told him to make a new file, rather than drop it in some other file
that's already full of unrealted junk :)
I did mean for him to put all the dawr_force_enable code in the file
though, not just the variable.
> As far as I understand, it is always compiled in, so can't we use
> another file like traps.o or setup-common.o or somewhere else ?
>
> Or just put an ifdef in arch/powerpc/kvm/book3s_hv_rmhandlers.S ?
> Because your fix will put dawr_force_enable on every build even the ones
> who don't need it at all.
We don't want to use an ifdef in the KVM code, because that would break
the case where you want to enable the DAWR for use by guests but the
host kernel doesn't have PERF support.
It shouldn't be in obj-y, it should at least be 64-bit only.
But it should be pretty trivial to create a config symbol for it, with
something like:
config DAWR_FORCE_ENABLE
def_bool y
depends on PERF || KVM
cheers
^ permalink raw reply
* Re: Crashes in linux-next on powerpc with CONFIG_PPC_KUAP and CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG
From: Michael Ellerman @ 2019-05-09 14:42 UTC (permalink / raw)
To: Petr Mladek; +Cc: linuxppc-dev, linux-kernel, Stephen Rothwell, Linus Torvalds
In-Reply-To: <20190509092942.ei4myfzt5dczuptj@pathway.suse.cz>
Petr Mladek <pmladek@suse.com> writes:
> On Wed 2019-05-08 00:54:51, Michael Ellerman wrote:
>> Hi folks,
>>
>> Just an FYI in case anyone else is seeing crashes very early in boot in
>> linux-next with the above config options.
>>
>> The problem is the combination of some new code called via printk(),
>> check_pointer() which calls probe_kernel_read(). That then calls
>> allow_user_access() (PPC_KUAP) and that uses mmu_has_feature() too early
>> (before we've patched features). With the JUMP_LABEL debug enabled that
>> causes us to call printk() & dump_stack() and we end up recursing and
>> overflowing the stack.
>
> Sigh, the check_pointer() stuff is in Linus's tree now, see
> the commit 3e5903eb9cff707301712 ("vsprintf: Prevent crash when
> dereferencing invalid pointers").
No worries.
>> Because it happens so early you don't get any output, just an apparently
>> dead system.
>>
>> The stack trace (which you don't see) is something like:
>>
>> ...
>> dump_stack+0xdc
>> probe_kernel_read+0x1a4
>> check_pointer+0x58
>> string+0x3c
>> vsnprintf+0x1bc
>> vscnprintf+0x20
>> printk_safe_log_store+0x7c
>> printk+0x40
>> dump_stack_print_info+0xbc
>> dump_stack+0x8
>> probe_kernel_read+0x1a4
>> probe_kernel_read+0x19c
>> check_pointer+0x58
>> string+0x3c
>> vsnprintf+0x1bc
>> vscnprintf+0x20
>> vprintk_store+0x6c
>> vprintk_emit+0xec
>> vprintk_func+0xd4
>> printk+0x40
>> cpufeatures_process_feature+0xc8
>> scan_cpufeatures_subnodes+0x380
>> of_scan_flat_dt_subnodes+0xb4
>> dt_cpu_ftrs_scan_callback+0x158
>> of_scan_flat_dt+0xf0
>> dt_cpu_ftrs_scan+0x3c
>> early_init_devtree+0x360
>> early_setup+0x9c
>>
>>
>> The simple fix is to use early_mmu_has_feature() in allow_user_access(),
>> but we'd rather not do that because it penalises all
>> copy_to/from_users() for the life of the system with the cost of the
>> runtime check vs the jump label. The irony is probe_kernel_read()
>> shouldn't be allowing user access at all, because we're reading the
>> kernel not userspace.
>
> I have tried to find a lightweight way for a safe reading of unknown
> kernel pointer. But I have not succeeded so far. I see only variants
> with user access. The user access is handled in arch-specific code
> and I do not see any variant without it.
>
> I am not sure on which level it should get fixed.
I sent a fix in powerpc code (sorry might have forgot to Cc you):
https://patchwork.ozlabs.org/patch/1097015/
I've merged that into the powerpc tree. I think it's too subtle for us
to have an ordering requirement that deep in the user copy code, it was
just a matter of time before it caused a problem, you were just unlucky
it was your patch that did :)
We'll eventually switch it back to using a jump label but make it safe
to call early in boot before we've detected features.
> Could you please send it to lkml to get a wider audience?
I see you also sent a fix, that looks like a safe default to me.
But as I said I'm happy with the powerpc fix, so there's no requirement
from us that your fix get merged.
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