* Re: VDSO ELF header
From: Dmitry Safonov @ 2021-03-26 15:13 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, Laurent Dufour; +Cc: linuxppc-dev
In-Reply-To: <c7e0568f-786b-2c1c-fb6d-cd804bc1b539@csgroup.eu>
Hello,
On 3/26/21 10:50 AM, Christophe Leroy wrote:
>
>
> Le 26/03/2021 à 11:46, Michael Ellerman a écrit :
>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>> Le 25/03/2021 à 17:56, Laurent Dufour a écrit :
>>>> Le 25/03/2021 à 17:46, Christophe Leroy a écrit :
>>>>> Le 25/03/2021 à 17:11, Laurent Dufour a écrit :
>>>>>> Since v5.11 and the changes you made to the VDSO code, it no more
>>>>>> exposing
>>>>>> the ELF header at the beginning of the VDSO mapping in user space.
>>>>>>
>>>>>> This is confusing CRIU which is checking for this ELF header cookie
>>>>>> (https://github.com/checkpoint-restore/criu/issues/1417).
>>>>>
>>>>> How does it do on other architectures ?
>>>>
>>>> Good question, I'll double check the CRIU code.
>>>
>>> On x86, there are 2 VDSO entries:
>>> 7ffff7fcb000-7ffff7fce000 r--p 00000000 00:00
>>> 0 [vvar]
>>> 7ffff7fce000-7ffff7fcf000 r-xp 00000000 00:00
>>> 0 [vdso]
>>>
>>> And the VDSO is starting with the ELF header.
>>>
>>>>>> I'm not an expert in loading and ELF part and reading the change
>>>>>> you made, I
>>>>>> can't identify how this could work now as I'm expecting the loader
>>>>>> to need
>>>>>> that ELF header to do the relocation.
>>>>>
>>>>> I think the loader is able to find it at the expected place.
>>>>
>>>> Actually, it seems the loader relies on the AUX vector
>>>> AT_SYSINFO_EHDR. I guess
>>>> CRIU should do the same.
>>>>
>>>>>>
>>>>>> From my investigation it seems that the first bytes of the VDSO
>>>>>> area are now
>>>>>> the vdso_arch_data.
>>>>>>
>>>>>> Is the ELF header put somewhere else?
>>>>>> How could the loader process the VDSO without that ELF header?
>>>>>>
>>>>>
>>>>> Like most other architectures, we now have the data section as
>>>>> first page and
>>>>> the text section follows. So you will likely find the elf header on
>>>>> the second
>>>>> page.
>>>
>>> I'm wondering if the data section you're refering to is the vvar
>>> section I can
>>> see on x86.
>>
>> Many of the other architectures have separate vm_special_mapping's for
>> the data page and the vdso binary, where the former is called "vvar".
>>
>> eg, s390:
>>
>> static struct vm_special_mapping vvar_mapping = {
>> .name = "[vvar]",
>> .fault = vvar_fault,
>> };
>>
>> static struct vm_special_mapping vdso_mapping = {
>> .name = "[vdso]",
>> .mremap = vdso_mremap,
>> };
>>
>>
>> I guess we probably should be doing that too.
>>
>
> Dmitry proposed the same, see
> https://github.com/0x7f454c46/linux/commit/783c7a2532d2219edbcf555cc540eab05f698d2a
>
>
> Discussion at https://github.com/checkpoint-restore/criu/issues/1417
Yeah, I didn't submit it officially to lkml because I couldn't test it
yet (and I usually don't send untested patches). The VM I have fails to
kexec and there's some difficulty to get serial console working, so I'd
appreciate if someone could either pick it up, or add tested-by.
Thanks,
Dmitry
^ permalink raw reply
* Re: [PATCH v3 11/17] riscv: Convert to GENERIC_CMDLINE
From: Rob Herring @ 2021-03-26 15:26 UTC (permalink / raw)
To: Christophe Leroy
Cc: open list:GENERIC INCLUDE/ASM HEADER FILES, devicetree,
microblaze, Daniel Gimpelevich, linux-xtensa, SH-Linux,
linuxppc-dev, X86 ML, linux-kernel@vger.kernel.org, nios2,
open list:MIPS, Openrisc, Andreas Schwab, linux-hexagon,
sparclinux, linux-riscv, Will Deacon, linux-arm-kernel,
Daniel Walker
In-Reply-To: <81a7e63f-57d4-5c81-acc5-35278fe5bb04@csgroup.eu>
On Fri, Mar 26, 2021 at 8:20 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 26/03/2021 à 15:08, Andreas Schwab a écrit :
> > On Mär 26 2021, Christophe Leroy wrote:
> >
> >> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> >> index f8f15332caa2..e7c91ee478d1 100644
> >> --- a/arch/riscv/kernel/setup.c
> >> +++ b/arch/riscv/kernel/setup.c
> >> @@ -20,6 +20,7 @@
> >> #include <linux/swiotlb.h>
> >> #include <linux/smp.h>
> >> #include <linux/efi.h>
> >> +#include <linux/cmdline.h>
> >>
> >> #include <asm/cpu_ops.h>
> >> #include <asm/early_ioremap.h>
> >> @@ -228,10 +229,8 @@ static void __init parse_dtb(void)
> >> }
> >>
> >> pr_err("No DTB passed to the kernel\n");
> >> -#ifdef CONFIG_CMDLINE_FORCE
> >> - strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> >> + cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE);
> >> pr_info("Forcing kernel command line to: %s\n", boot_command_line);
> >
> > Shouldn't that message become conditional in some way?
> >
>
> You are right, I did something similar on ARM but looks like I missed it on RISCV.
How is this hunk even useful? Under what conditions can you boot
without a DTB? Even with a built-in DTB, the DT cmdline handling would
be called.
Rob
^ permalink raw reply
* Re: [PATCH v3 1/1] hotplug-cpu.c: show 'last online CPU' error in dlpar_cpu_offline()
From: Andrew Donnellan @ 2021-03-26 15:30 UTC (permalink / raw)
To: Daniel Henrique Barboza, linuxppc-dev; +Cc: dja
In-Reply-To: <20210326141954.236323-2-danielhb413@gmail.com>
On 27/3/21 1:19 am, Daniel Henrique Barboza wrote:
> One of the reasons that dlpar_cpu_offline can fail is when attempting to
> offline the last online CPU of the kernel. This can be observed in a
> pseries QEMU guest that has hotplugged CPUs. If the user offlines all
> other CPUs of the guest, and a hotplugged CPU is now the last online
> CPU, trying to reclaim it will fail. See [1] for an example.
>
> The current error message in this situation returns rc with -EBUSY and a
> generic explanation, e.g.:
>
> pseries-hotplug-cpu: Failed to offline CPU PowerPC,POWER9, rc: -16
>
> EBUSY can be caused by other conditions, such as cpu_hotplug_disable
> being true. Throwing a more specific error message for this case,
> instead of just "Failed to offline CPU", makes it clearer that the error
> is in fact a known error situation instead of other generic/unknown
> cause.
>
> This patch adds a 'last online' check in dlpar_cpu_offline() to catch
> the 'last online CPU' offline error, returning a more informative error
> message:
>
> pseries-hotplug-cpu: Unable to remove last online CPU PowerPC,POWER9
>
> [1] https://bugzilla.redhat.com/1911414
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Thanks for addressing the issues in Daniel's review.
I haven't tested it, but this patch looks sensible enough to me.
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH 2/4] exec: remove compat_do_execve
From: Andreas Schwab @ 2021-03-26 15:42 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linux-s390, linux-parisc, Arnd Bergmann, Brian Gerst,
x86, linux-mips, linux-kernel, linux-fsdevel, Luis Chamberlain,
Eric W. Biederman, sparclinux, linuxppc-dev, linux-arm-kernel,
Al Viro
In-Reply-To: <20210326143831.1550030-3-hch@lst.de>
On Mär 26 2021, Christoph Hellwig wrote:
> Just call compat_do_execve instead.
ITYM compat_do_execveat here.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH v3 01/17] cmdline: Add generic function to build command line.
From: Rob Herring @ 2021-03-26 15:42 UTC (permalink / raw)
To: Christophe Leroy
Cc: open list:GENERIC INCLUDE/ASM HEADER FILES, devicetree,
microblaze, Daniel Gimpelevich, linux-xtensa, SH-Linux,
Will Deacon, X86 ML, linux-kernel@vger.kernel.org, nios2,
open list:MIPS, Openrisc, linux-hexagon, sparclinux, linux-riscv,
linuxppc-dev, linux-arm-kernel, Daniel Walker
In-Reply-To: <878228ad88df38f8914c7aa25dede3ed05c50f48.1616765869.git.christophe.leroy@csgroup.eu>
On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
> This code provides architectures with a way to build command line
> based on what is built in the kernel and what is handed over by the
> bootloader, based on selected compile-time options.
Note that I have this patch pending:
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210316193820.3137-1-alex@ghiti.fr/
It's going to need to be adapted for this. I've held off applying to
see if this gets settled.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> v3:
> - Addressed comments from Will
> - Added capability to have src == dst
> ---
> include/linux/cmdline.h | 57 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
> create mode 100644 include/linux/cmdline.h
>
> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
> new file mode 100644
> index 000000000000..dea87edd41be
> --- /dev/null
> +++ b/include/linux/cmdline.h
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_CMDLINE_H
> +#define _LINUX_CMDLINE_H
> +
> +#include <linux/string.h>
> +
> +/* Allow architectures to override strlcat, powerpc can't use strings so early */
> +#ifndef cmdline_strlcat
> +#define cmdline_strlcat strlcat
> +#endif
> +
> +/*
> + * This function will append or prepend a builtin command line to the command
> + * line provided by the bootloader. Kconfig options can be used to alter
> + * the behavior of this builtin command line.
> + * @dst: The destination of the final appended/prepended string.
> + * @src: The starting string or NULL if there isn't one.
> + * @len: the length of dest buffer.
> + */
> +static __always_inline void __cmdline_build(char *dst, const char *src, size_t len)
> +{
> + if (!len || src == dst)
> + return;
> +
> + if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src) {
> + dst[0] = 0;
> + cmdline_strlcat(dst, CONFIG_CMDLINE, len);
> + return;
> + }
> +
> + if (dst != src)
> + dst[0] = 0;
> +
> + if (IS_ENABLED(CONFIG_CMDLINE_PREPEND))
> + cmdline_strlcat(dst, CONFIG_CMDLINE " ", len);
> +
> + cmdline_strlcat(dst, src, len);
> +
> + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND))
Should be APPEND.
> + cmdline_strlcat(dst, " " CONFIG_CMDLINE, len);
> +}
> +
> +#define cmdline_build(dst, src, len) do { \
Perhaps a comment why we need this to be a define.
> + char *__c_dst = (dst); \
> + const char *__c_src = (src); \
> + \
> + if (__c_src == __c_dst) { \
> + static char __c_tmp[COMMAND_LINE_SIZE] __initdata = ""; \
> + \
> + cmdline_strlcat(__c_tmp, __c_src, COMMAND_LINE_SIZE); \
> + __cmdline_build(__c_dst, __c_tmp, (len)); \
> + } else { \
> + __cmdline_build(__c_dst, __c_src, (len)); \
> + } \
> +} while (0)
> +
> +#endif /* _LINUX_CMDLINE_H */
> --
> 2.25.0
>
^ permalink raw reply
* Re: [PATCH v3 05/17] arm: Convert to GENERIC_CMDLINE
From: Rob Herring @ 2021-03-26 15:47 UTC (permalink / raw)
To: Christophe Leroy
Cc: open list:GENERIC INCLUDE/ASM HEADER FILES, devicetree,
microblaze, Daniel Gimpelevich, linux-xtensa, SH-Linux,
Will Deacon, X86 ML, linux-kernel@vger.kernel.org, nios2,
open list:MIPS, Openrisc, linux-hexagon, sparclinux, linux-riscv,
linuxppc-dev, linux-arm-kernel, Daniel Walker
In-Reply-To: <7362e4f6a5f5b79e6ad3fd3cec3183a4a283f7fc.1616765870.git.christophe.leroy@csgroup.eu>
On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
> This converts the architecture to GENERIC_CMDLINE.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/arm/Kconfig | 38 +----------------------------------
> arch/arm/kernel/atags_parse.c | 15 +++++---------
> 2 files changed, 6 insertions(+), 47 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 5da96f5df48f..67bc75f2da81 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -50,6 +50,7 @@ config ARM
> select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY
> select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI
> select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> + select GENERIC_CMDLINE if ATAGS
Don't we need this enabled for !ATAGS (i.e. DT boot)?
Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE?
Rob
^ permalink raw reply
* Re: [PATCH v3 05/17] arm: Convert to GENERIC_CMDLINE
From: Christophe Leroy @ 2021-03-26 15:49 UTC (permalink / raw)
To: Rob Herring
Cc: open list:GENERIC INCLUDE/ASM HEADER FILES, devicetree,
microblaze, Daniel Gimpelevich, linux-xtensa, SH-Linux,
Will Deacon, X86 ML, linux-kernel@vger.kernel.org, nios2,
open list:MIPS, Openrisc, linux-hexagon, sparclinux, linux-riscv,
linuxppc-dev, linux-arm-kernel, Daniel Walker
In-Reply-To: <CAL_Jsq+LF-s5K4Jwd5jCHrU8271L5WCiGb0tR7aTUa8ddHF1YQ@mail.gmail.com>
Le 26/03/2021 à 16:47, Rob Herring a écrit :
> On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>> This converts the architecture to GENERIC_CMDLINE.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/arm/Kconfig | 38 +----------------------------------
>> arch/arm/kernel/atags_parse.c | 15 +++++---------
>> 2 files changed, 6 insertions(+), 47 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 5da96f5df48f..67bc75f2da81 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -50,6 +50,7 @@ config ARM
>> select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY
>> select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI
>> select GENERIC_CLOCKEVENTS_BROADCAST if SMP
>> + select GENERIC_CMDLINE if ATAGS
>
> Don't we need this enabled for !ATAGS (i.e. DT boot)?
>
> Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE?
>
Don't know.
Today ARM has:
choice
prompt "Kernel command line type" if CMDLINE != ""
default CMDLINE_FROM_BOOTLOADER
depends on ATAGS
Christophe
^ permalink raw reply
* Re: [PATCH V2 3/5] tools/perf: Add powerpc support for PERF_SAMPLE_WEIGHT_STRUCT
From: Arnaldo @ 2021-03-26 15:50 UTC (permalink / raw)
To: Athira Rajeev, Jiri Olsa
Cc: ravi.bangoria, Madhavan Srinivasan, peterz, linux-kernel, acme,
linux-perf-users, jolsa, kjain, linuxppc-dev, kan.liang
In-Reply-To: <80EE46ED-9007-4CB7-9A52-A7A2ADC616C6@linux.vnet.ibm.com>
On March 26, 2021 12:23:04 PM GMT-03:00, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
>
>
>On 25-Mar-2021, at 1:13 AM, Jiri Olsa <jolsa@redhat.com> wrote:
>
>On Mon, Mar 22, 2021 at 10:57:25AM -0400, Athira Rajeev wrote:
>
>Add arch specific arch_evsel__set_sample_weight() to set the new
>sample type for powerpc.
>
>Add arch specific arch_perf_parse_sample_weight() to store the
>sample->weight values depending on the sample type applied.
>if the new sample type (PERF_SAMPLE_WEIGHT_STRUCT) is applied,
>store only the lower 32 bits to sample->weight. If sample type
>is 'PERF_SAMPLE_WEIGHT', store the full 64-bit to sample->weight.
>
>Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>---
>tools/perf/arch/powerpc/util/Build | 2 ++
>tools/perf/arch/powerpc/util/event.c | 32
>++++++++++++++++++++++++++++++++
>tools/perf/arch/powerpc/util/evsel.c | 8 ++++++++
>3 files changed, 42 insertions(+)
>create mode 100644 tools/perf/arch/powerpc/util/event.c
>create mode 100644 tools/perf/arch/powerpc/util/evsel.c
>
>diff --git a/tools/perf/arch/powerpc/util/Build
>b/tools/perf/arch/powerpc/util/Build
>index b7945e5a543b..8a79c4126e5b 100644
>--- a/tools/perf/arch/powerpc/util/Build
>+++ b/tools/perf/arch/powerpc/util/Build
>@@ -4,6 +4,8 @@ perf-y += kvm-stat.o
>perf-y += perf_regs.o
>perf-y += mem-events.o
>perf-y += sym-handling.o
>+perf-y += evsel.o
>+perf-y += event.o
>
>perf-$(CONFIG_DWARF) += dwarf-regs.o
>perf-$(CONFIG_DWARF) += skip-callchain-idx.o
>diff --git a/tools/perf/arch/powerpc/util/event.c
>b/tools/perf/arch/powerpc/util/event.c
>new file mode 100644
>index 000000000000..f49d32c2c8ae
>--- /dev/null
>+++ b/tools/perf/arch/powerpc/util/event.c
>@@ -0,0 +1,32 @@
>+// SPDX-License-Identifier: GPL-2.0
>+#include <linux/types.h>
>+#include <linux/string.h>
>+#include <linux/zalloc.h>
>+
>+#include "../../../util/event.h"
>+#include "../../../util/synthetic-events.h"
>+#include "../../../util/machine.h"
>+#include "../../../util/tool.h"
>+#include "../../../util/map.h"
>+#include "../../../util/debug.h"
>
>
>nit, just #include "utils/...h" should work no?
>
>other than that, the patchset looks ok to me
>
>Acked-by: Jiri Olsa <jolsa@redhat.com>
>
>
>
>Hi Jiri, Arnaldo
>
>Thanks for reviewing the patch set.
>I checked that, just using "utils/...h" also works.
>Below is the change which I verified. Since the patches are presently
>merged in
>https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=tmp.perf/core,
>
>can you please suggest how can we go about this change ?
I'll fix it up here,
Thanks for the patch.
- Arnaldo
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH v3 01/17] cmdline: Add generic function to build command line.
From: Christophe Leroy @ 2021-03-26 15:55 UTC (permalink / raw)
To: Rob Herring
Cc: open list:GENERIC INCLUDE/ASM HEADER FILES, devicetree,
microblaze, Daniel Gimpelevich, linux-xtensa, SH-Linux,
Will Deacon, X86 ML, linux-kernel@vger.kernel.org, nios2,
open list:MIPS, Openrisc, linux-hexagon, sparclinux, linux-riscv,
linuxppc-dev, linux-arm-kernel, Daniel Walker
In-Reply-To: <CAL_JsqKr3xekKSo3DtQvOOw_VoGC=FUTagZGY5g=CGGGdUZSMQ@mail.gmail.com>
Le 26/03/2021 à 16:42, Rob Herring a écrit :
> On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>> This code provides architectures with a way to build command line
>> based on what is built in the kernel and what is handed over by the
>> bootloader, based on selected compile-time options.
>
> Note that I have this patch pending:
>
> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20210316193820.3137-1-alex@ghiti.fr/
>
> It's going to need to be adapted for this. I've held off applying to
> see if this gets settled.
good point.
Hope we can't have things like
option="beautiful weather"
>
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> v3:
>> - Addressed comments from Will
>> - Added capability to have src == dst
>> ---
>> include/linux/cmdline.h | 57 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 57 insertions(+)
>> create mode 100644 include/linux/cmdline.h
>>
>> diff --git a/include/linux/cmdline.h b/include/linux/cmdline.h
>> new file mode 100644
>> index 000000000000..dea87edd41be
>> --- /dev/null
>> +++ b/include/linux/cmdline.h
>> @@ -0,0 +1,57 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _LINUX_CMDLINE_H
>> +#define _LINUX_CMDLINE_H
>> +
>> +#include <linux/string.h>
>> +
>> +/* Allow architectures to override strlcat, powerpc can't use strings so early */
>> +#ifndef cmdline_strlcat
>> +#define cmdline_strlcat strlcat
>> +#endif
>> +
>> +/*
>> + * This function will append or prepend a builtin command line to the command
>> + * line provided by the bootloader. Kconfig options can be used to alter
>> + * the behavior of this builtin command line.
>> + * @dst: The destination of the final appended/prepended string.
>> + * @src: The starting string or NULL if there isn't one.
>> + * @len: the length of dest buffer.
>> + */
>> +static __always_inline void __cmdline_build(char *dst, const char *src, size_t len)
>> +{
>> + if (!len || src == dst)
>> + return;
>> +
>> + if (IS_ENABLED(CONFIG_CMDLINE_FORCE) || !src) {
>> + dst[0] = 0;
>> + cmdline_strlcat(dst, CONFIG_CMDLINE, len);
>> + return;
>> + }
>> +
>> + if (dst != src)
>> + dst[0] = 0;
>> +
>> + if (IS_ENABLED(CONFIG_CMDLINE_PREPEND))
>> + cmdline_strlcat(dst, CONFIG_CMDLINE " ", len);
>> +
>> + cmdline_strlcat(dst, src, len);
>> +
>> + if (IS_ENABLED(CONFIG_CMDLINE_EXTEND))
>
> Should be APPEND.
Not yet. For the time being all architectures use EXTEND only.
In patch 3 it is changed to:
- if (IS_ENABLED(CONFIG_CMDLINE_EXTEND))
+ if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || IS_ENABLED(CONFIG_CMDLINE_APPEND))
Then in last patch, I forgot but I should have done:
- if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) || IS_ENABLED(CONFIG_CMDLINE_APPEND))
+ if (IS_ENABLED(CONFIG_CMDLINE_APPEND))
>
>> + cmdline_strlcat(dst, " " CONFIG_CMDLINE, len);
>> +}
>> +
>> +#define cmdline_build(dst, src, len) do { \
>
> Perhaps a comment why we need this to be a define.
Probably we don't need anymore as I finally decided to use COMMAND_LINE_SIZE instead of 'len' as the
size of the temporary buffer.
>
>> + char *__c_dst = (dst); \
>> + const char *__c_src = (src); \
>> + \
>> + if (__c_src == __c_dst) { \
>> + static char __c_tmp[COMMAND_LINE_SIZE] __initdata = ""; \
>> + \
>> + cmdline_strlcat(__c_tmp, __c_src, COMMAND_LINE_SIZE); \
>> + __cmdline_build(__c_dst, __c_tmp, (len)); \
>> + } else { \
>> + __cmdline_build(__c_dst, __c_src, (len)); \
>> + } \
>> +} while (0)
>> +
>> +#endif /* _LINUX_CMDLINE_H */
>> --
>> 2.25.0
>>
Christophe
^ permalink raw reply
* Re: VDSO ELF header
From: Christophe Leroy @ 2021-03-26 16:11 UTC (permalink / raw)
To: Dmitry Safonov, Michael Ellerman, Laurent Dufour; +Cc: linuxppc-dev
In-Reply-To: <e8c2b4b5-f0b6-945e-4a42-c2308dd719a2@gmail.com>
Le 26/03/2021 à 16:13, Dmitry Safonov a écrit :
> Hello,
>
> On 3/26/21 10:50 AM, Christophe Leroy wrote:
>>
>>
>> Le 26/03/2021 à 11:46, Michael Ellerman a écrit :
>>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>>> Le 25/03/2021 à 17:56, Laurent Dufour a écrit :
>>>>> Le 25/03/2021 à 17:46, Christophe Leroy a écrit :
>>>>>> Le 25/03/2021 à 17:11, Laurent Dufour a écrit :
>>>>>>> Since v5.11 and the changes you made to the VDSO code, it no more
>>>>>>> exposing
>>>>>>> the ELF header at the beginning of the VDSO mapping in user space.
>>>>>>>
>>>>>>> This is confusing CRIU which is checking for this ELF header cookie
>>>>>>> (https://github.com/checkpoint-restore/criu/issues/1417).
>>>>>>
>>>>>> How does it do on other architectures ?
>>>>>
>>>>> Good question, I'll double check the CRIU code.
>>>>
>>>> On x86, there are 2 VDSO entries:
>>>> 7ffff7fcb000-7ffff7fce000 r--p 00000000 00:00
>>>> 0 [vvar]
>>>> 7ffff7fce000-7ffff7fcf000 r-xp 00000000 00:00
>>>> 0 [vdso]
>>>>
>>>> And the VDSO is starting with the ELF header.
>>>>
>>>>>>> I'm not an expert in loading and ELF part and reading the change
>>>>>>> you made, I
>>>>>>> can't identify how this could work now as I'm expecting the loader
>>>>>>> to need
>>>>>>> that ELF header to do the relocation.
>>>>>>
>>>>>> I think the loader is able to find it at the expected place.
>>>>>
>>>>> Actually, it seems the loader relies on the AUX vector
>>>>> AT_SYSINFO_EHDR. I guess
>>>>> CRIU should do the same.
>>>>>
>>>>>>>
>>>>>>> From my investigation it seems that the first bytes of the VDSO
>>>>>>> area are now
>>>>>>> the vdso_arch_data.
>>>>>>>
>>>>>>> Is the ELF header put somewhere else?
>>>>>>> How could the loader process the VDSO without that ELF header?
>>>>>>>
>>>>>>
>>>>>> Like most other architectures, we now have the data section as
>>>>>> first page and
>>>>>> the text section follows. So you will likely find the elf header on
>>>>>> the second
>>>>>> page.
>>>>
>>>> I'm wondering if the data section you're refering to is the vvar
>>>> section I can
>>>> see on x86.
>>>
>>> Many of the other architectures have separate vm_special_mapping's for
>>> the data page and the vdso binary, where the former is called "vvar".
>>>
>>> eg, s390:
>>>
>>> static struct vm_special_mapping vvar_mapping = {
>>> .name = "[vvar]",
>>> .fault = vvar_fault,
>>> };
>>>
>>> static struct vm_special_mapping vdso_mapping = {
>>> .name = "[vdso]",
>>> .mremap = vdso_mremap,
>>> };
>>>
>>>
>>> I guess we probably should be doing that too.
>>>
>>
>> Dmitry proposed the same, see
>> https://github.com/0x7f454c46/linux/commit/783c7a2532d2219edbcf555cc540eab05f698d2a
>>
>>
>> Discussion at https://github.com/checkpoint-restore/criu/issues/1417
>
> Yeah, I didn't submit it officially to lkml because I couldn't test it
> yet (and I usually don't send untested patches). The VM I have fails to
> kexec and there's some difficulty to get serial console working, so I'd
> appreciate if someone could either pick it up, or add tested-by.
>
Just to let everyone know, while testing your patch with selftest I encountered the following Oops.
But I also have it without your patch thought.
root@vgoip:~# ./sigreturn_vdso
test: sigreturn_vdso
tags: git_version:v5.12-rc4-1553-gc31141d460e6
VDSO is at 0x104000-0x10bfff (32768 bytes)
Signal delivered OK with VDSO mapped
VDSO moved to 0x77bf4000-0x77bfbfff (32768 bytes)
Signal delivered OK with VDSO moved
Unmapped VDSO
[ 1855.444371] Kernel attempted to read user page (7ff9ff30) - exploit attempt? (uid: 0)
[ 1855.459404] BUG: Unable to handle kernel data access on read at 0x7ff9ff30
[ 1855.466188] Faulting instruction address: 0xc00111d4
[ 1855.471099] Oops: Kernel access of bad area, sig: 11 [#1]
[ 1855.476428] BE PAGE_SIZE=16K PREEMPT CMPC885
[ 1855.480702] SAF3000 DIE NOTIFICATION
[ 1855.484184] CPU: 0 PID: 362 Comm: sigreturn_vdso Not tainted
5.12.0-rc4-s3k-dev-01553-gc31141d460e6 #4811
[ 1855.493644] NIP: c00111d4 LR: c0005a28 CTR: 00000000
[ 1855.498634] REGS: cadb3dd0 TRAP: 0300 Not tainted (5.12.0-rc4-s3k-dev-01553-gc31141d460e6)
[ 1855.507068] MSR: 00009032 <EE,ME,IR,DR,RI> CR: 48000884 XER: 20000000
[ 1855.513866] DAR: 7ff9ff30 DSISR: 88000000
[ 1855.513866] GPR00: c0007788 cadb3e90 c28dc000 7ff9ff30 7ff9ff40 000004e0 7ff9fd50 00000000
[ 1855.513866] GPR08: 00000001 00000001 7ff9ff30 00000000 28000282 1001b7e8 100a0920 00000000
[ 1855.513866] GPR16: 100cac0c 100b0000 102883a4 10289685 100d0000 100d0000 100d0000 100b2e9e
[ 1855.513866] GPR24: ffffffff 102883c8 00000000 7ff9ff38 cadb3f40 cadb3ec8 c28dc000 00000000
[ 1855.552767] NIP [c00111d4] flush_icache_range+0x90/0xb4
[ 1855.557932] LR [c0005a28] handle_signal32+0x1bc/0x1c4
[ 1855.562925] Call Trace:
[ 1855.565332] [cadb3e90] [100d0000] 0x100d0000 (unreliable)
[ 1855.570666] [cadb3ec0] [c0007788] do_notify_resume+0x260/0x314
[ 1855.576432] [cadb3f20] [c000c764] syscall_exit_prepare+0x120/0x184
[ 1855.582542] [cadb3f30] [c00100b4] ret_from_syscall+0xc/0x28
[ 1855.588050] --- interrupt: c00 at 0xfe807f8
[ 1855.592183] NIP: 0fe807f8 LR: 10001048 CTR: c0139378
[ 1855.597174] REGS: cadb3f40 TRAP: 0c00 Not tainted (5.12.0-rc4-s3k-dev-01553-gc31141d460e6)
[ 1855.605607] MSR: 0000d032 <EE,PR,ME,IR,DR,RI> CR: 28000282 XER: 20000000
[ 1855.612664]
[ 1855.612664] GPR00: 00000025 7ffa0230 77c09690 00000000 0000000a 28000282 00000001 0ff03a38
[ 1855.612664] GPR08: 0000d032 00000328 c28dc000 00000009 88000282 1001b7e8 100a0920 00000000
[ 1855.612664] GPR16: 100cac0c 100b0000 102883a4 10289685 100d0000 100d0000 100d0000 100b2e9e
[ 1855.612664] GPR24: ffffffff 102883c8 00000000 77bff628 10002358 10010000 1000210c 00008000
[ 1855.648894] NIP [0fe807f8] 0xfe807f8
[ 1855.652426] LR [10001048] 0x10001048
[ 1855.655954] --- interrupt: c00
[ 1855.658969] Instruction dump:
[ 1855.661893] 38630010 7c001fac 38630010 4200fff0 7c0004ac 4c00012c 4e800020 7c001fac
[ 1855.669811] 2c0a0000 38630010 4082ffcc 4bffffe4 <7c00186c> 2c070000 39430010 4082ff8c
[ 1855.677910] ---[ end trace f071a5587092b3aa ]---
[ 1855.682462]
Remapped the stack executable
!! child died by signal 11
failure: sigreturn_vdso
^ permalink raw reply
* Re: [PATCH 3/4] exec: simplify the compat syscall handling
From: Al Viro @ 2021-03-26 16:12 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linux-s390, linux-parisc, Arnd Bergmann, Brian Gerst,
x86, linux-mips, linux-kernel, linux-fsdevel, Luis Chamberlain,
Eric W. Biederman, sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20210326143831.1550030-4-hch@lst.de>
On Fri, Mar 26, 2021 at 03:38:30PM +0100, Christoph Hellwig wrote:
> -static const char __user *get_user_arg_ptr(struct user_arg_ptr argv, int nr)
> +static const char __user *
> +get_user_arg_ptr(const char __user *const __user *argv, int nr)
> {
> - const char __user *native;
> -
> -#ifdef CONFIG_COMPAT
> - if (unlikely(argv.is_compat)) {
> + if (in_compat_syscall()) {
> + const compat_uptr_t __user *compat_argv =
> + compat_ptr((unsigned long)argv);
> compat_uptr_t compat;
>
> - if (get_user(compat, argv.ptr.compat + nr))
> + if (get_user(compat, compat_argv + nr))
> return ERR_PTR(-EFAULT);
> -
> return compat_ptr(compat);
> - }
> -#endif
> -
> - if (get_user(native, argv.ptr.native + nr))
> - return ERR_PTR(-EFAULT);
> + } else {
> + const char __user *native;
>
> - return native;
> + if (get_user(native, argv + nr))
> + return ERR_PTR(-EFAULT);
> + return native;
> + }
> }
Yecchhh.... So you have in_compat_syscall() called again and again, for
each argument in the list? I agree that current version is fucking ugly,
but I really hate that approach ;-/
^ permalink raw reply
* Re: VDSO ELF header
From: Dmitry Safonov @ 2021-03-26 16:32 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, Laurent Dufour; +Cc: linuxppc-dev
In-Reply-To: <3ce8811a-9216-6164-cdf2-974b13e400d3@csgroup.eu>
On 3/26/21 4:11 PM, Christophe Leroy wrote:
[..]
>>>
>>> Dmitry proposed the same, see
>>> https://github.com/0x7f454c46/linux/commit/783c7a2532d2219edbcf555cc540eab05f698d2a
>>>
>>>
>>>
>>> Discussion at https://github.com/checkpoint-restore/criu/issues/1417
>>
>> Yeah, I didn't submit it officially to lkml because I couldn't test it
>> yet (and I usually don't send untested patches). The VM I have fails to
>> kexec and there's some difficulty to get serial console working, so I'd
>> appreciate if someone could either pick it up, or add tested-by.
>>
>
> Just to let everyone know, while testing your patch with selftest I
> encountered the following Oops. But I also have it without your patch
> thought.
Thank you, Christophe!
>
> root@vgoip:~# ./sigreturn_vdso
> test: sigreturn_vdso
> tags: git_version:v5.12-rc4-1553-gc31141d460e6
> VDSO is at 0x104000-0x10bfff (32768 bytes)
> Signal delivered OK with VDSO mapped
> VDSO moved to 0x77bf4000-0x77bfbfff (32768 bytes)
> Signal delivered OK with VDSO moved
> Unmapped VDSO
> [ 1855.444371] Kernel attempted to read user page (7ff9ff30) - exploit
> attempt? (uid: 0)
> [ 1855.459404] BUG: Unable to handle kernel data access on read at
> 0x7ff9ff30
> [ 1855.466188] Faulting instruction address: 0xc00111d4
> [ 1855.471099] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 1855.476428] BE PAGE_SIZE=16K PREEMPT CMPC885
> [ 1855.480702] SAF3000 DIE NOTIFICATION
> [ 1855.484184] CPU: 0 PID: 362 Comm: sigreturn_vdso Not tainted
> 5.12.0-rc4-s3k-dev-01553-gc31141d460e6 #4811
> [ 1855.493644] NIP: c00111d4 LR: c0005a28 CTR: 00000000
> [ 1855.498634] REGS: cadb3dd0 TRAP: 0300 Not tainted
> (5.12.0-rc4-s3k-dev-01553-gc31141d460e6)
> [ 1855.507068] MSR: 00009032 <EE,ME,IR,DR,RI> CR: 48000884 XER: 20000000
> [ 1855.513866] DAR: 7ff9ff30 DSISR: 88000000
> [ 1855.513866] GPR00: c0007788 cadb3e90 c28dc000 7ff9ff30 7ff9ff40
> 000004e0 7ff9fd50 00000000
> [ 1855.513866] GPR08: 00000001 00000001 7ff9ff30 00000000 28000282
> 1001b7e8 100a0920 00000000
> [ 1855.513866] GPR16: 100cac0c 100b0000 102883a4 10289685 100d0000
> 100d0000 100d0000 100b2e9e
> [ 1855.513866] GPR24: ffffffff 102883c8 00000000 7ff9ff38 cadb3f40
> cadb3ec8 c28dc000 00000000
> [ 1855.552767] NIP [c00111d4] flush_icache_range+0x90/0xb4
> [ 1855.557932] LR [c0005a28] handle_signal32+0x1bc/0x1c4
> [ 1855.562925] Call Trace:
> [ 1855.565332] [cadb3e90] [100d0000] 0x100d0000 (unreliable)
> [ 1855.570666] [cadb3ec0] [c0007788] do_notify_resume+0x260/0x314
> [ 1855.576432] [cadb3f20] [c000c764] syscall_exit_prepare+0x120/0x184
> [ 1855.582542] [cadb3f30] [c00100b4] ret_from_syscall+0xc/0x28
> [ 1855.588050] --- interrupt: c00 at 0xfe807f8
> [ 1855.592183] NIP: 0fe807f8 LR: 10001048 CTR: c0139378
> [ 1855.597174] REGS: cadb3f40 TRAP: 0c00 Not tainted
> (5.12.0-rc4-s3k-dev-01553-gc31141d460e6)
> [ 1855.605607] MSR: 0000d032 <EE,PR,ME,IR,DR,RI> CR: 28000282 XER:
> 20000000
> [ 1855.612664]
> [ 1855.612664] GPR00: 00000025 7ffa0230 77c09690 00000000 0000000a
> 28000282 00000001 0ff03a38
> [ 1855.612664] GPR08: 0000d032 00000328 c28dc000 00000009 88000282
> 1001b7e8 100a0920 00000000
> [ 1855.612664] GPR16: 100cac0c 100b0000 102883a4 10289685 100d0000
> 100d0000 100d0000 100b2e9e
> [ 1855.612664] GPR24: ffffffff 102883c8 00000000 77bff628 10002358
> 10010000 1000210c 00008000
> [ 1855.648894] NIP [0fe807f8] 0xfe807f8
> [ 1855.652426] LR [10001048] 0x10001048
> [ 1855.655954] --- interrupt: c00
> [ 1855.658969] Instruction dump:
> [ 1855.661893] 38630010 7c001fac 38630010 4200fff0 7c0004ac 4c00012c
> 4e800020 7c001fac
> [ 1855.669811] 2c0a0000 38630010 4082ffcc 4bffffe4 <7c00186c> 2c070000
> 39430010 4082ff8c
> [ 1855.677910] ---[ end trace f071a5587092b3aa ]---
> [ 1855.682462]
> Remapped the stack executable
> !! child died by signal 11
> failure: sigreturn_vdso
Yes, it seems unrelated.
Probably, a bit hacky solution to this one could be:
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -911,7 +911,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t
*oldset,
}
user_write_access_end();
- if (tramp == (unsigned long)mctx->mc_pad)
+ if ((tramp == (unsigned long)mctx->mc_pad) && access_ok(tramp,
2*sizeof(unsigned long)))
flush_icache_range(tramp, tramp + 2 * sizeof(unsigned
long));
regs->link = tramp;
--
But it's up to you, it seems power-related.
Thanks,
Dmitry
^ permalink raw reply
* RE: [PATCH 3/4] exec: simplify the compat syscall handling
From: David Laight @ 2021-03-26 16:44 UTC (permalink / raw)
To: 'Al Viro', Christoph Hellwig
Cc: linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
linux-parisc@vger.kernel.org, Arnd Bergmann, Brian Gerst,
x86@kernel.org, linux-mips@vger.kernel.org,
linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,
Luis Chamberlain, Eric W. Biederman,
linux-fsdevel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <YF4H58gozyNkoCeO@zeniv-ca.linux.org.uk>
From: Al Viro
> Sent: 26 March 2021 16:12
>
> On Fri, Mar 26, 2021 at 03:38:30PM +0100, Christoph Hellwig wrote:
>
> > +static const char __user *
> > +get_user_arg_ptr(const char __user *const __user *argv, int nr)
> > {
> > + if (in_compat_syscall()) {
> > + const compat_uptr_t __user *compat_argv =
> > + compat_ptr((unsigned long)argv);
> > compat_uptr_t compat;
> >
> > + if (get_user(compat, compat_argv + nr))
> > return ERR_PTR(-EFAULT);
> > return compat_ptr(compat);
> > + } else {
> > + const char __user *native;
> >
> > + if (get_user(native, argv + nr))
> > + return ERR_PTR(-EFAULT);
> > + return native;
> > + }
> > }
>
> Yecchhh.... So you have in_compat_syscall() called again and again, for
> each argument in the list? I agree that current version is fucking ugly,
> but I really hate that approach ;-/
Especially since in_compat_syscall() isn't entirely trivial on x86-64.
Probably all in the noise for 'exec', but all the bits do add up.
You may not want separate get_user() on some architectures either.
The user_access_begin/end aren't cheap.
OTOH if you call copy_from_user() you get hit by the stupid
additional costs of 'user copy hardening'.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: VDSO ELF header
From: Christophe Leroy @ 2021-03-26 17:07 UTC (permalink / raw)
To: Dmitry Safonov, Michael Ellerman, Laurent Dufour; +Cc: linuxppc-dev
In-Reply-To: <edf2547f-4705-764b-dadd-3ec1d2a6b8b3@gmail.com>
Le 26/03/2021 à 17:32, Dmitry Safonov a écrit :
> On 3/26/21 4:11 PM, Christophe Leroy wrote:
> [..]
>>>>
>>>> Dmitry proposed the same, see
>>>> https://github.com/0x7f454c46/linux/commit/783c7a2532d2219edbcf555cc540eab05f698d2a
>>>>
>>>>
>>>>
>>>> Discussion at https://github.com/checkpoint-restore/criu/issues/1417
>>>
>>> Yeah, I didn't submit it officially to lkml because I couldn't test it
>>> yet (and I usually don't send untested patches). The VM I have fails to
>>> kexec and there's some difficulty to get serial console working, so I'd
>>> appreciate if someone could either pick it up, or add tested-by.
>>>
>>
>> Just to let everyone know, while testing your patch with selftest I
>> encountered the following Oops. But I also have it without your patch
>> thought.
>
> Thank you, Christophe!
>
>>
>> root@vgoip:~# ./sigreturn_vdso
>> test: sigreturn_vdso
>> tags: git_version:v5.12-rc4-1553-gc31141d460e6
>> VDSO is at 0x104000-0x10bfff (32768 bytes)
>> Signal delivered OK with VDSO mapped
>> VDSO moved to 0x77bf4000-0x77bfbfff (32768 bytes)
>> Signal delivered OK with VDSO moved
>> Unmapped VDSO
>> [ 1855.444371] Kernel attempted to read user page (7ff9ff30) - exploit
>> attempt? (uid: 0)
>> [ 1855.459404] BUG: Unable to handle kernel data access on read at
>> 0x7ff9ff30
>> [ 1855.466188] Faulting instruction address: 0xc00111d4
>> [ 1855.471099] Oops: Kernel access of bad area, sig: 11 [#1]
>> [ 1855.476428] BE PAGE_SIZE=16K PREEMPT CMPC885
>> [ 1855.480702] SAF3000 DIE NOTIFICATION
>> [ 1855.484184] CPU: 0 PID: 362 Comm: sigreturn_vdso Not tainted
>> 5.12.0-rc4-s3k-dev-01553-gc31141d460e6 #4811
>> [ 1855.493644] NIP: c00111d4 LR: c0005a28 CTR: 00000000
>> [ 1855.498634] REGS: cadb3dd0 TRAP: 0300 Not tainted
>> (5.12.0-rc4-s3k-dev-01553-gc31141d460e6)
>> [ 1855.507068] MSR: 00009032 <EE,ME,IR,DR,RI> CR: 48000884 XER: 20000000
>> [ 1855.513866] DAR: 7ff9ff30 DSISR: 88000000
>> [ 1855.513866] GPR00: c0007788 cadb3e90 c28dc000 7ff9ff30 7ff9ff40
>> 000004e0 7ff9fd50 00000000
>> [ 1855.513866] GPR08: 00000001 00000001 7ff9ff30 00000000 28000282
>> 1001b7e8 100a0920 00000000
>> [ 1855.513866] GPR16: 100cac0c 100b0000 102883a4 10289685 100d0000
>> 100d0000 100d0000 100b2e9e
>> [ 1855.513866] GPR24: ffffffff 102883c8 00000000 7ff9ff38 cadb3f40
>> cadb3ec8 c28dc000 00000000
>> [ 1855.552767] NIP [c00111d4] flush_icache_range+0x90/0xb4
>> [ 1855.557932] LR [c0005a28] handle_signal32+0x1bc/0x1c4
>> [ 1855.562925] Call Trace:
>> [ 1855.565332] [cadb3e90] [100d0000] 0x100d0000 (unreliable)
>> [ 1855.570666] [cadb3ec0] [c0007788] do_notify_resume+0x260/0x314
>> [ 1855.576432] [cadb3f20] [c000c764] syscall_exit_prepare+0x120/0x184
>> [ 1855.582542] [cadb3f30] [c00100b4] ret_from_syscall+0xc/0x28
>> [ 1855.588050] --- interrupt: c00 at 0xfe807f8
>> [ 1855.592183] NIP: 0fe807f8 LR: 10001048 CTR: c0139378
>> [ 1855.597174] REGS: cadb3f40 TRAP: 0c00 Not tainted
>> (5.12.0-rc4-s3k-dev-01553-gc31141d460e6)
>> [ 1855.605607] MSR: 0000d032 <EE,PR,ME,IR,DR,RI> CR: 28000282 XER:
>> 20000000
>> [ 1855.612664]
>> [ 1855.612664] GPR00: 00000025 7ffa0230 77c09690 00000000 0000000a
>> 28000282 00000001 0ff03a38
>> [ 1855.612664] GPR08: 0000d032 00000328 c28dc000 00000009 88000282
>> 1001b7e8 100a0920 00000000
>> [ 1855.612664] GPR16: 100cac0c 100b0000 102883a4 10289685 100d0000
>> 100d0000 100d0000 100b2e9e
>> [ 1855.612664] GPR24: ffffffff 102883c8 00000000 77bff628 10002358
>> 10010000 1000210c 00008000
>> [ 1855.648894] NIP [0fe807f8] 0xfe807f8
>> [ 1855.652426] LR [10001048] 0x10001048
>> [ 1855.655954] --- interrupt: c00
>> [ 1855.658969] Instruction dump:
>> [ 1855.661893] 38630010 7c001fac 38630010 4200fff0 7c0004ac 4c00012c
>> 4e800020 7c001fac
>> [ 1855.669811] 2c0a0000 38630010 4082ffcc 4bffffe4 <7c00186c> 2c070000
>> 39430010 4082ff8c
>> [ 1855.677910] ---[ end trace f071a5587092b3aa ]---
>> [ 1855.682462]
>> Remapped the stack executable
>> !! child died by signal 11
>> failure: sigreturn_vdso
>
> Yes, it seems unrelated.
> Probably, a bit hacky solution to this one could be:
>
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -911,7 +911,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t
> *oldset,
> }
> user_write_access_end();
>
> - if (tramp == (unsigned long)mctx->mc_pad)
> + if ((tramp == (unsigned long)mctx->mc_pad) && access_ok(tramp,
> 2*sizeof(unsigned long)))
> flush_icache_range(tramp, tramp + 2 * sizeof(unsigned
> long));
>
> regs->link = tramp;
> --
>
> But it's up to you, it seems power-related.
>
No, the problem is that user access has to be allowed for the flush()
A hacky solution would be to call user_access_begin() , will test that later
Christophe
^ permalink raw reply
* Re: VDSO ELF header
From: Dmitry Safonov @ 2021-03-26 17:11 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, Laurent Dufour; +Cc: linuxppc-dev
In-Reply-To: <4904769a-be42-cdd2-1d97-8b1889f7f723@csgroup.eu>
On 3/26/21 5:07 PM, Christophe Leroy wrote:
> No, the problem is that user access has to be allowed for the flush()
>
> A hacky solution would be to call user_access_begin() , will test that
> later
Yeah, cool.
Will it be fine if I send the vvar patch with your Tested-by?
Thanks,
Dmitry
^ permalink raw reply
* Re: [PATCH v2 0/8] Implement EBPF on powerpc32
From: Andrii Nakryiko @ 2021-03-26 18:09 UTC (permalink / raw)
To: Christophe Leroy
Cc: Song Liu, Daniel Borkmann, john fastabend, Andrii Nakryiko,
Alexei Starovoitov, naveen.n.rao, Networking, Paul Mackerras,
sandipan, KP Singh, Yonghong Song, bpf, linuxppc-dev, Martin Lau,
open list
In-Reply-To: <86028d25-c3fe-3765-f7c3-12448523405a@csgroup.eu>
On Fri, Mar 26, 2021 at 7:42 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 22/03/2021 à 18:53, Andrii Nakryiko a écrit :
> > On Mon, Mar 22, 2021 at 9:37 AM Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> >>
> >> This series implements extended BPF on powerpc32. For the implementation
> >> details, see the patch before the last.
> >>
> >> The following operations are not implemented:
> >>
> >> case BPF_ALU64 | BPF_DIV | BPF_X: /* dst /= src */
> >> case BPF_ALU64 | BPF_MOD | BPF_X: /* dst %= src */
> >> case BPF_STX | BPF_XADD | BPF_DW: /* *(u64 *)(dst + off) += src */
> >>
> >> The following operations are only implemented for power of two constants:
> >>
> >> case BPF_ALU64 | BPF_MOD | BPF_K: /* dst %= imm */
> >> case BPF_ALU64 | BPF_DIV | BPF_K: /* dst /= imm */
> >>
> >> Below are the results on a powerpc 885:
> >> - with the patch, with and without bpf_jit_enable
> >> - without the patch, with bpf_jit_enable (ie with CBPF)
> >>
> >> With the patch, with bpf_jit_enable = 1 :
> >>
> >> [ 60.826529] test_bpf: Summary: 378 PASSED, 0 FAILED, [354/366 JIT'ed]
> >> [ 60.832505] test_bpf: test_skb_segment: Summary: 2 PASSED, 0 FAILED
> >>
> >> With the patch, with bpf_jit_enable = 0 :
> >>
> >> [ 75.186337] test_bpf: Summary: 378 PASSED, 0 FAILED, [0/366 JIT'ed]
> >> [ 75.192325] test_bpf: test_skb_segment: Summary: 2 PASSED, 0 FAILED
> >>
> >> Without the patch, with bpf_jit_enable = 1 :
> >>
> >> [ 186.112429] test_bpf: Summary: 371 PASSED, 7 FAILED, [119/366 JIT'ed]
> >>
> >> Couldn't run test_progs because it doesn't build (clang 11 crashes during the build).
> >
> > Can you please try checking out the latest clang from sources and use
> > that one instead?
>
> The crash is fixed, it builds one step more, then fails at:
>
> [root@PC-server-ldb bpf]# make CROSS_COMPILE=ppc-linux- ARCH=powerpc V=1
> /root/gen_ldb/linux-powerpc/tools/testing/selftests/bpf/host-tools/sbin/bpftool gen skeleton
> /root/gen_ldb/linux-powerpc/tools/testing/selftests/bpf/atomic_bounds.o >
> /root/gen_ldb/linux-powerpc/tools/testing/selftests/bpf/atomic_bounds.skel.h
> libbpf: elf: endianness mismatch in atomic_bounds.
> Error: failed to open BPF object file: Endian mismatch
>
> I'm cross-building on x86 for powerpc/32
yeah, I'm not sure selftests/bpf supports cross-compiling. bpftool got
some patches recently to enable cross-compiling, but probably not
selftests/bpf.
>
> [root@PC-server-ldb bpf]# file atomic_bounds.o
> atomic_bounds.o: ELF 64-bit MSB relocatable, eBPF, version 1 (SYSV), with debug_info, not stripped
>
> Christophe
^ permalink raw reply
* Re: [PATCH v3 05/17] arm: Convert to GENERIC_CMDLINE
From: Rob Herring @ 2021-03-26 18:22 UTC (permalink / raw)
To: Christophe Leroy, Nicolas Pitre
Cc: open list:GENERIC INCLUDE/ASM HEADER FILES, devicetree,
microblaze, Daniel Gimpelevich, linux-xtensa, SH-Linux,
Will Deacon, X86 ML, linux-kernel@vger.kernel.org, nios2,
open list:MIPS, Openrisc, linux-hexagon, sparclinux, linux-riscv,
linuxppc-dev, linux-arm-kernel, Daniel Walker
In-Reply-To: <c18ef8f7-8e79-a9d3-3853-f8b992a4fc93@csgroup.eu>
+Nico who added the line in question.
On Fri, Mar 26, 2021 at 9:50 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 26/03/2021 à 16:47, Rob Herring a écrit :
> > On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> >>
> >> This converts the architecture to GENERIC_CMDLINE.
> >>
> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> >> ---
> >> arch/arm/Kconfig | 38 +----------------------------------
> >> arch/arm/kernel/atags_parse.c | 15 +++++---------
> >> 2 files changed, 6 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> index 5da96f5df48f..67bc75f2da81 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -50,6 +50,7 @@ config ARM
> >> select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY
> >> select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI
> >> select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> >> + select GENERIC_CMDLINE if ATAGS
> >
> > Don't we need this enabled for !ATAGS (i.e. DT boot)?
> >
> > Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE?
> >
>
> Don't know.
>
> Today ARM has:
>
> choice
> prompt "Kernel command line type" if CMDLINE != ""
> default CMDLINE_FROM_BOOTLOADER
> depends on ATAGS
I think that's a mistake. In a DT only case (no ATAGS), we'll get
different behaviour (in fdt.c) depending if CONFIG_ATAGS is enabled or
not. Note that at the time (2012) the above was added, the DT code
only supported CONFIG_CMDLINE and CONFIG_CMDLINE_FORCE.
CONFIG_CMDLINE_EXTEND was only added in 2016. And that has different
behavior for ATAGS vs. DT. In summary, it's a mess. We should drop the
depends either before this patch or just as part of this patch IMO.
I'd go with the latter given CONFIG_ATAGS is default y and enabled for
common configs. Without that, it looks like CONFIG_CMDLINE disappears
from menuconfig for at91_dt_defconfig.
Also, I think this code should be refactored a bit to eliminate
default_command_line. Instead, we should just save a pointer to the
ATAGS command line string, and then call cmdline_build here instead of
doing the extra copy:
/* parse_early_param needs a boot_command_line */
strlcpy(boot_command_line, from, COMMAND_LINE_SIZE);
Rob
^ permalink raw reply
* Re: VDSO ELF header
From: Christophe Leroy @ 2021-03-26 18:40 UTC (permalink / raw)
To: Dmitry Safonov, Michael Ellerman, Laurent Dufour; +Cc: linuxppc-dev
In-Reply-To: <46e49770-04ee-2455-f2f7-b3915ceea38a@gmail.com>
Le 26/03/2021 à 18:11, Dmitry Safonov a écrit :
> On 3/26/21 5:07 PM, Christophe Leroy wrote:
>> No, the problem is that user access has to be allowed for the flush()
>>
>> A hacky solution would be to call user_access_begin() , will test that
>> later
>
> Yeah, cool.
>
> Will it be fine if I send the vvar patch with your Tested-by?
>
Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
With the user access fixed on the flush, it sigreturn_vdso selftest is a success. I'll send a patch
for it in the coming days.
What is the status of your series which adds generic vdso_base tracking ?
Christophe
^ permalink raw reply
* Re: VDSO ELF header
From: Dmitry Safonov @ 2021-03-26 18:54 UTC (permalink / raw)
To: Christophe Leroy, Michael Ellerman, Laurent Dufour; +Cc: linuxppc-dev
In-Reply-To: <ec257b8b-d611-bbf4-86f3-bb0b758dbeaf@csgroup.eu>
On 3/26/21 6:40 PM, Christophe Leroy wrote:
>
>
> Le 26/03/2021 à 18:11, Dmitry Safonov a écrit :
>> On 3/26/21 5:07 PM, Christophe Leroy wrote:
>>> No, the problem is that user access has to be allowed for the flush()
>>>
>>> A hacky solution would be to call user_access_begin() , will test that
>>> later
>>
>> Yeah, cool.
>>
>> Will it be fine if I send the vvar patch with your Tested-by?
>>
>
> Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Thank you!
I'll properly submit it shortly..
> With the user access fixed on the flush, it sigreturn_vdso selftest is a
> success. I'll send a patch for it in the coming days.
Nice!
> What is the status of your series which adds generic vdso_base tracking ?
Yeah, I was doing a new version of patches and I always was unsatisfied
by the result and stuck between "good" and "best" (more code rewriting).
And then had some other work to finish.
I'll try to finish and send it next week, thanks for pinging :-)
--
Dmitry
^ permalink raw reply
* [PATCH] powerpc/vdso: Separate vvar vma from vdso
From: Dmitry Safonov @ 2021-03-26 19:17 UTC (permalink / raw)
To: linux-kernel
Cc: Dmitry Safonov, Dmitry Safonov, Andrei Vagin, Paul Mackerras,
stable, Andy Lutomirski, Laurent Dufour, linuxppc-dev
Since commit 511157ab641e ("powerpc/vdso: Move vdso datapage up front")
VVAR page is in front of the VDSO area. In result it breaks CRIU
(Checkpoint Restore In Userspace) [1], where CRIU expects that "[vdso]"
from /proc/../maps points at ELF/vdso image, rather than at VVAR data page.
Laurent made a patch to keep CRIU working (by reading aux vector).
But I think it still makes sence to separate two mappings into different
VMAs. It will also make ppc64 less "special" for userspace and as
a side-bonus will make VVAR page un-writable by debugger (which previously
would COW page and can be unexpected).
I opportunistically Cc stable on it: I understand that usually such
stuff isn't a stable material, but that will allow us in CRIU have
one workaround less that is needed just for one release (v5.11) on
one platform (ppc64), which we otherwise have to maintain.
I wouldn't go as far as to say that the commit 511157ab641e is ABI
regression as no other userspace got broken, but I'd really appreciate
if it gets backported to v5.11 after v5.12 is released, so as not
to complicate already non-simple CRIU-vdso code. Thanks!
Cc: Andrei Vagin <avagin@gmail.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Laurent Dufour <ldufour@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: stable@vger.kernel.org # v5.11
[1]: https://github.com/checkpoint-restore/criu/issues/1417
Signed-off-by: Dmitry Safonov <dima@arista.com>
Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/mmu_context.h | 2 +-
arch/powerpc/kernel/vdso.c | 54 +++++++++++++++++++-------
2 files changed, 40 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 652ce85f9410..4bc45d3ed8b0 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -263,7 +263,7 @@ extern void arch_exit_mmap(struct mm_struct *mm);
static inline void arch_unmap(struct mm_struct *mm,
unsigned long start, unsigned long end)
{
- unsigned long vdso_base = (unsigned long)mm->context.vdso - PAGE_SIZE;
+ unsigned long vdso_base = (unsigned long)mm->context.vdso;
if (start <= vdso_base && vdso_base < end)
mm->context.vdso = NULL;
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index e839a906fdf2..b14907209822 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -55,10 +55,10 @@ static int vdso_mremap(const struct vm_special_mapping *sm, struct vm_area_struc
{
unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
- if (new_size != text_size + PAGE_SIZE)
+ if (new_size != text_size)
return -EINVAL;
- current->mm->context.vdso = (void __user *)new_vma->vm_start + PAGE_SIZE;
+ current->mm->context.vdso = (void __user *)new_vma->vm_start;
return 0;
}
@@ -73,6 +73,10 @@ static int vdso64_mremap(const struct vm_special_mapping *sm, struct vm_area_str
return vdso_mremap(sm, new_vma, &vdso64_end - &vdso64_start);
}
+static struct vm_special_mapping vvar_spec __ro_after_init = {
+ .name = "[vvar]",
+};
+
static struct vm_special_mapping vdso32_spec __ro_after_init = {
.name = "[vdso]",
.mremap = vdso32_mremap,
@@ -89,11 +93,11 @@ static struct vm_special_mapping vdso64_spec __ro_after_init = {
*/
static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
{
- struct mm_struct *mm = current->mm;
+ unsigned long vdso_size, vdso_base, mappings_size;
struct vm_special_mapping *vdso_spec;
+ unsigned long vvar_size = PAGE_SIZE;
+ struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
- unsigned long vdso_size;
- unsigned long vdso_base;
if (is_32bit_task()) {
vdso_spec = &vdso32_spec;
@@ -110,8 +114,8 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
vdso_base = 0;
}
- /* Add a page to the vdso size for the data page */
- vdso_size += PAGE_SIZE;
+ mappings_size = vdso_size + vvar_size;
+ mappings_size += (VDSO_ALIGNMENT - 1) & PAGE_MASK;
/*
* pick a base address for the vDSO in process space. We try to put it
@@ -119,9 +123,7 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
* and end up putting it elsewhere.
* Add enough to the size so that the result can be aligned.
*/
- vdso_base = get_unmapped_area(NULL, vdso_base,
- vdso_size + ((VDSO_ALIGNMENT - 1) & PAGE_MASK),
- 0, 0);
+ vdso_base = get_unmapped_area(NULL, vdso_base, mappings_size, 0, 0);
if (IS_ERR_VALUE(vdso_base))
return vdso_base;
@@ -133,7 +135,13 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
* install_special_mapping or the perf counter mmap tracking code
* will fail to recognise it as a vDSO.
*/
- mm->context.vdso = (void __user *)vdso_base + PAGE_SIZE;
+ mm->context.vdso = (void __user *)vdso_base + vvar_size;
+
+ vma = _install_special_mapping(mm, vdso_base, vvar_size,
+ VM_READ | VM_MAYREAD | VM_IO |
+ VM_DONTDUMP | VM_PFNMAP, &vvar_spec);
+ if (IS_ERR(vma))
+ return PTR_ERR(vma);
/*
* our vma flags don't have VM_WRITE so by default, the process isn't
@@ -145,9 +153,12 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
* It's fine to use that for setting breakpoints in the vDSO code
* pages though.
*/
- vma = _install_special_mapping(mm, vdso_base, vdso_size,
+ vma = _install_special_mapping(mm, vdso_base + vvar_size, vdso_size,
VM_READ | VM_EXEC | VM_MAYREAD |
VM_MAYWRITE | VM_MAYEXEC, vdso_spec);
+ if (IS_ERR(vma))
+ do_munmap(mm, vdso_base, vvar_size, NULL);
+
return PTR_ERR_OR_ZERO(vma);
}
@@ -249,11 +260,22 @@ static struct page ** __init vdso_setup_pages(void *start, void *end)
if (!pagelist)
panic("%s: Cannot allocate page list for VDSO", __func__);
- pagelist[0] = virt_to_page(vdso_data);
-
for (i = 0; i < pages; i++)
- pagelist[i + 1] = virt_to_page(start + i * PAGE_SIZE);
+ pagelist[i] = virt_to_page(start + i * PAGE_SIZE);
+
+ return pagelist;
+}
+
+static struct page ** __init vvar_setup_pages(void)
+{
+ struct page **pagelist;
+ /* .pages is NULL-terminated */
+ pagelist = kcalloc(2, sizeof(struct page *), GFP_KERNEL);
+ if (!pagelist)
+ panic("%s: Cannot allocate page list for VVAR", __func__);
+
+ pagelist[0] = virt_to_page(vdso_data);
return pagelist;
}
@@ -295,6 +317,8 @@ static int __init vdso_init(void)
if (IS_ENABLED(CONFIG_PPC64))
vdso64_spec.pages = vdso_setup_pages(&vdso64_start, &vdso64_end);
+ vvar_spec.pages = vvar_setup_pages();
+
smp_wmb();
return 0;
--
2.31.0
^ permalink raw reply related
* Re: [PATCH] powerpc/mm/book3s64: Use the correct storage key value when calling H_PROTECT
From: Murilo Opsfelder Araújo @ 2021-03-26 20:39 UTC (permalink / raw)
To: linuxppc-dev, mpe, Aneesh Kumar K.V; +Cc: Aneesh Kumar K.V
In-Reply-To: <20210326070755.304625-1-aneesh.kumar@linux.ibm.com>
On Friday, March 26, 2021 4:07:55 AM -03 Aneesh Kumar K.V wrote:
> H_PROTECT expect the flag value to include
> flags: AVPN, pp0, pp1, pp2, key0-key4, Noexec, CMO Option flags
>
> This patch updates hpte_updatepp() to fetch the storage key value from the
> linux page table and use the same in H_PROTECT hcall.
>
> native_hpte_updatepp() is not updated because the kernel doesn't clear the
> existing storage key value there. The kernel also doesn't use
> hpte_updatepp() callback for updating storage keys.
>
> This fixes the below kernel crash observed with KUAP enabled.
>
> BUG: Unable to handle kernel data access on write at 0xc009fffffc440000
> Faulting instruction address: 0xc0000000000b7030
> Key fault AMR: 0xfcffffffffffffff IAMR: 0xc0000077bc498100
> Found HPTE: v = 0x40070adbb6fffc05 r = 0x1ffffffffff1194
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> .........
> CFAR: c000000000010100 DAR: c009fffffc440000 DSISR: 02200000 IRQMASK: 0
> ..........
> NIP [c0000000000b7030] memset+0x68/0x104
> LR [c0000000003ef00c] pcpu_alloc+0x54c/0xb50
> Call Trace:
> [c00000001c7534f0] [c0000000003ef01c] pcpu_alloc+0x55c/0xb50 (unreliable)
> [c00000001c753600] [c0000000008bb214] blk_stat_alloc_callback+0x94/0x150
> [c00000001c753650] [c0000000008b7a04]
> blk_mq_init_allocated_queue+0x64/0x560 [c00000001c7536b0]
> [c0000000008b8024] blk_mq_init_queue+0x54/0xb0 [c00000001c7536e0]
> [c000000000b87650] scsi_mq_alloc_queue+0x30/0xa0 [c00000001c753710]
> [c000000000b88b2c] scsi_alloc_sdev+0x1cc/0x300 [c00000001c7537b0]
> [c000000000b897b0] scsi_probe_and_add_lun+0xb50/0x1020 [c00000001c753950]
> [c000000000b8a35c] __scsi_scan_target+0x17c/0x790 [c00000001c753a80]
> [c000000000b8ab90] scsi_scan_channel+0x90/0xe0 [c00000001c753ad0]
> [c000000000b8ae48] scsi_scan_host_selected+0x148/0x1f0 [c00000001c753b60]
> [c000000000b8b31c] do_scan_async+0x2c/0x2a0
> [c00000001c753be0] [c000000000187a18] async_run_entry_fn+0x78/0x220
> [c00000001c753c70] [c000000000176a74] process_one_work+0x264/0x540
> [c00000001c753d10] [c000000000177338] worker_thread+0xa8/0x600
> [c00000001c753da0] [c0000000001807b0] kthread+0x190/0x1a0
> [c00000001c753e10] [c00000000000d8f0] ret_from_kernel_thread+0x5c/0x6c
>
> With KUAP enabled the kernel uses storage key 3 for all its translations.
> But as shown by the debug print, in this specific case we have the hash
> page table entry created with key value 0.
>
> [ 2.249497] Found HPTE: v = 0x40070adbb6fffc05 r = 0x1ffffffffff1194
>
> and DSISR indicates a key fault.
>
> This can happen due to parallel fault on the same EA by different CPUs
>
> CPU 0 CPU 1
> fault on X
>
> H_PAGE_BUSY set
> fault on X
>
> finish fault handling and
> clear H_PAGE_BUSY
> check for H_PAGE_BUSY
> continue with fault
handling.
>
> This implies CPU1 will end up calling hpte_updatepp for address X
> and the kernel updated the hash pte entry with key 0
>
> Fixes: d94b827e89dc ("powerpc/book3s64/kuap: Use Key 3 for kernel mapping
> with hash translation")
>
> Debugged-by: Michael Ellerman <mpe@ellerman.id.au>
> Reported-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
I've tested this on top of commit db24726bfefa68c606947a86132591568a06bfb4
("Merge tag 'integrity-v5.12-fix' of git://git.kernel.org/pub/scm/linux/
kernel/git/zohar/linux-integrity"),
and the reported issue did not manifest.
Thank you, Michael and Aneesh, for the help.
Tested-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
--
Murilo
^ permalink raw reply
* Re: [PATCH V2 3/5] tools/perf: Add powerpc support for PERF_SAMPLE_WEIGHT_STRUCT
From: Athira Rajeev @ 2021-03-26 15:23 UTC (permalink / raw)
To: Jiri Olsa
Cc: ravi.bangoria, Madhavan Srinivasan, peterz, linux-kernel, acme,
linux-perf-users, jolsa, kjain, linuxppc-dev, kan.liang
In-Reply-To: <YFuWb3S8p0ZGjmGu@krava>
[-- Attachment #1: Type: text/html, Size: 6519 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] exec: simplify the compat syscall handling
From: Eric W. Biederman @ 2021-03-26 21:22 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linux-s390, linux-parisc, Arnd Bergmann, Brian Gerst,
x86, linux-mips, linux-kernel, linux-fsdevel, Luis Chamberlain,
Al Viro, sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20210326143831.1550030-4-hch@lst.de>
Christoph Hellwig <hch@lst.de> writes:
> diff --git a/fs/exec.c b/fs/exec.c
> index 06e07278b456fa..b34c1eb9e7ad8e 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -391,47 +391,34 @@ static int bprm_mm_init(struct linux_binprm *bprm)
> return err;
> }
>
> -struct user_arg_ptr {
> -#ifdef CONFIG_COMPAT
> - bool is_compat;
> -#endif
> - union {
> - const char __user *const __user *native;
> -#ifdef CONFIG_COMPAT
> - const compat_uptr_t __user *compat;
> -#endif
> - } ptr;
> -};
> -
> -static const char __user *get_user_arg_ptr(struct user_arg_ptr argv, int nr)
> +static const char __user *
> +get_user_arg_ptr(const char __user *const __user *argv, int nr)
> {
> - const char __user *native;
> -
> -#ifdef CONFIG_COMPAT
> - if (unlikely(argv.is_compat)) {
> + if (in_compat_syscall()) {
> + const compat_uptr_t __user *compat_argv =
> + compat_ptr((unsigned long)argv);
Ouch! Passing a pointer around as the wrong type through the kernel!
Perhaps we should reduce everything to do_execveat and
do_execveat_compat. Then there would be no need for anything
to do anything odd with the pointer types.
I think the big change would be to factor out a copy_string out
of copy_strings, that performs all of the work once we know the proper
pointer value.
Casting pointers from one type to another scares me as one mistake means
we are doing something wrong and probably exploitable.
Eric
> compat_uptr_t compat;
>
> - if (get_user(compat, argv.ptr.compat + nr))
> + if (get_user(compat, compat_argv + nr))
> return ERR_PTR(-EFAULT);
> -
> return compat_ptr(compat);
> - }
> -#endif
> -
> - if (get_user(native, argv.ptr.native + nr))
> - return ERR_PTR(-EFAULT);
> + } else {
> + const char __user *native;
>
> - return native;
> + if (get_user(native, argv + nr))
> + return ERR_PTR(-EFAULT);
> + return native;
> + }
> }
>
^ permalink raw reply
* Re: [PATCH V2 1/5] powerpc/perf: Expose processor pipeline stage cycles using PERF_SAMPLE_WEIGHT_STRUCT
From: Michael Ellerman @ 2021-03-27 13:14 UTC (permalink / raw)
To: Arnaldo, Peter Zijlstra, Arnaldo Carvalho de Melo
Cc: ravi.bangoria, Athira Rajeev, kjain, linux-kernel,
linux-perf-users, Madhavan Srinivasan, jolsa, linuxppc-dev,
kan.liang
In-Reply-To: <2BAC42AE-6BD3-45EF-8867-1A15F25FE80B@gmail.com>
Arnaldo <arnaldo.melo@gmail.com> writes:
> On March 25, 2021 11:38:01 AM GMT-03:00, Peter Zijlstra <peterz@infradead.org> wrote:
>>On Thu, Mar 25, 2021 at 10:01:35AM -0300, Arnaldo Carvalho de Melo
>>wrote:.
>>> > > Also for CPU_FTR_ARCH_31, capture the two cycle counter
>>information in
>>> > > two 16 bit fields of perf_sample_weight structure.
>>> >
>>> > Changes looks fine to me.
>>> >
>>> > Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
>>>
>>> So who will process the kernel bits? I'm merging the tooling parts,
>>
>>I was sorta expecting these to go through the powerpc tree. Let me know
>>if you want them in tip/perf/core instead.
>
> Shouldn't matter by which tree it gets upstream, as long as it gets picked :-)
I plan to take them, just haven't got around to it yet :}
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/vdso: Separate vvar vma from vdso
From: Christophe Leroy @ 2021-03-27 17:19 UTC (permalink / raw)
To: Dmitry Safonov, linux-kernel
Cc: Dmitry Safonov, stable, Andrei Vagin, Paul Mackerras,
Andy Lutomirski, Laurent Dufour, linuxppc-dev
In-Reply-To: <20210326191720.138155-1-dima@arista.com>
Le 26/03/2021 à 20:17, Dmitry Safonov a écrit :
> Since commit 511157ab641e ("powerpc/vdso: Move vdso datapage up front")
> VVAR page is in front of the VDSO area. In result it breaks CRIU
> (Checkpoint Restore In Userspace) [1], where CRIU expects that "[vdso]"
> from /proc/../maps points at ELF/vdso image, rather than at VVAR data page.
> Laurent made a patch to keep CRIU working (by reading aux vector).
> But I think it still makes sence to separate two mappings into different
> VMAs. It will also make ppc64 less "special" for userspace and as
> a side-bonus will make VVAR page un-writable by debugger (which previously
> would COW page and can be unexpected).
>
> I opportunistically Cc stable on it: I understand that usually such
> stuff isn't a stable material, but that will allow us in CRIU have
> one workaround less that is needed just for one release (v5.11) on
> one platform (ppc64), which we otherwise have to maintain.
Why is that a workaround, and why for one release only ? I think the solution proposed by Laurentto
use the aux vector AT_SYSINFO_EHDR should work with any past and future release.
> I wouldn't go as far as to say that the commit 511157ab641e is ABI
> regression as no other userspace got broken, but I'd really appreciate
> if it gets backported to v5.11 after v5.12 is released, so as not
> to complicate already non-simple CRIU-vdso code. Thanks!
>
> Cc: Andrei Vagin <avagin@gmail.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Laurent Dufour <ldufour@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: stable@vger.kernel.org # v5.11
> [1]: https://github.com/checkpoint-restore/criu/issues/1417
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
I tested it with sifreturn_vdso selftest and it worked, because that selftest doesn't involve VDSO data.
But if I do a mremap() on the VDSO text vma without remapping VVAR to keep the same distance between
the two vmas, gettimeofday() crashes. The reason is that the code obtains the address of the data by
calculating a fix difference from its own address with the below macro, the delta being resolved at
link time:
.macro get_datapage ptr
bcl 20, 31, .+4
999:
mflr \ptr
#if CONFIG_PPC_PAGE_SHIFT > 14
addis \ptr, \ptr, (_vdso_datapage - 999b)@ha
#endif
addi \ptr, \ptr, (_vdso_datapage - 999b)@l
.endm
So the datapage needs to remain at the same distance from the code at all time.
Wondering how the other architectures do to have two independant VMAs and be able to move one
independantly of the other.
Christophe
^ 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