Re: [PATCH V2 2/2] ASoC: fsl_esai: recover the channel swap after xrun
> > > + > > + /* restore registers by regcache_sync */ > > + fsl_esai_register_restore(esai_priv); > > + > > + regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR, > > +ESAI_xCR_xPR_MASK, 0); > > + regmap_update_bits(esai_priv->regmap, REG_ESAI_RCR, > > +ESAI_xCR_xPR_MASK, 0); > > And just for curious, can (or shall) we stuff this personal reset to the > reset() > function? I found this one is a part of the reset routine being mentioned in > the RM -- it was done after ESAI reset is done via ECR register. > There is a problem to do this, TPR/RPR need to be clear after configure the control register. (TCCR, TCR). So it seems not only one place (reset function) need to be changed. Best regards Wang shengjiu
[PATCH] powerpc/prom: fix use-after-free on cpu_to_chip_id()
The np variable is still being used after the of_node_put() call, which may result in use-after-free. We fix this issue by calling of_node_put() after the last usage. Fixes: 3eb906c6b6c1 ("powerpc: Make cpu_to_chip_id() available when SMP=n") Signed-off-by: Wen Yang Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: Christophe Leroy Cc: Andrew Morton Cc: Mike Rapoport Cc: Greg Kroah-Hartman Cc: "Aneesh Kumar K.V" Cc: Thomas Gleixner Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-ker...@vger.kernel.org --- arch/powerpc/kernel/prom.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index 7159e79..ad87b94 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -872,13 +872,15 @@ EXPORT_SYMBOL(of_get_ibm_chip_id); int cpu_to_chip_id(int cpu) { struct device_node *np; + int id; np = of_get_cpu_node(cpu, NULL); if (!np) return -1; + id = of_get_ibm_chip_id(np); of_node_put(np); - return of_get_ibm_chip_id(np); + return id; } EXPORT_SYMBOL(cpu_to_chip_id); -- 2.9.5
[PATCH] powerpc/83xx: fix use-after-free on mpc831x_usb_cfg()
The np variable is still being used after the of_node_put() call, which may result in use-after-free. We fix this issue by calling of_node_put() after the last usage. This patatch also do some cleanup. Fixes: fd066e850351 ("powerpc/mpc8308: fix USB DR controller initialization") Signed-off-by: Wen Yang Cc: Scott Wood Cc: Kumar Gala Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-ker...@vger.kernel.org --- arch/powerpc/platforms/83xx/usb.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/platforms/83xx/usb.c b/arch/powerpc/platforms/83xx/usb.c index 3d247d7..56b36fa 100644 --- a/arch/powerpc/platforms/83xx/usb.c +++ b/arch/powerpc/platforms/83xx/usb.c @@ -158,14 +158,11 @@ int mpc831x_usb_cfg(void) iounmap(immap); - of_node_put(immr_node); - /* Map USB SOC space */ ret = of_address_to_resource(np, 0, &res); - if (ret) { - of_node_put(np); - return ret; - } + if (ret) + goto out_put_node; + usb_regs = ioremap(res.start, resource_size(&res)); /* Using on-chip PHY */ @@ -174,7 +171,7 @@ int mpc831x_usb_cfg(void) u32 refsel; if (of_device_is_compatible(immr_node, "fsl,mpc8308-immr")) - goto out; + goto out_unmap; if (of_device_is_compatible(immr_node, "fsl,mpc8315-immr")) refsel = CONTROL_REFSEL_24MHZ; @@ -201,8 +198,10 @@ int mpc831x_usb_cfg(void) ret = -EINVAL; } -out: +out_unmap: iounmap(usb_regs); +out_put_node: + of_node_put(immr_node); of_node_put(np); return ret; } -- 2.9.5
[PATCH] powerpc: fix use-after-free on fixup_port_irq()
There is a possible use-after-free issue in the fixup_port_irq(): 460 static void __init fixup_port_irq(int index, 461 struct device_node *np, 462 struct plat_serial8250_port *port) 463 { ... 469 if (!virq && legacy_serial_infos[index].irq_check_parent) { 470 np = of_get_parent(np); --> modified here. ... 474 of_node_put(np); ---> released here 475 } ... 481 #ifdef CONFIG_SERIAL_8250_FSL 482 if (of_device_is_compatible(np, "fsl,ns16550")) --> dereferenced here ... 484 #endif 485 } We solve this problem by introducing a new parent_np variable. Fixes: 9deaa53ac7fa ("serial: add irq handler for Freescale 16550 errata.") Signed-off-by: Wen Yang Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: Rob Herring Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-ker...@vger.kernel.org --- arch/powerpc/kernel/legacy_serial.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c index 7cea597..0105f3e 100644 --- a/arch/powerpc/kernel/legacy_serial.c +++ b/arch/powerpc/kernel/legacy_serial.c @@ -461,17 +461,18 @@ static void __init fixup_port_irq(int index, struct device_node *np, struct plat_serial8250_port *port) { + struct device_node *parent_np; unsigned int virq; DBG("fixup_port_irq(%d)\n", index); virq = irq_of_parse_and_map(np, 0); if (!virq && legacy_serial_infos[index].irq_check_parent) { - np = of_get_parent(np); - if (np == NULL) + parent_np = of_get_parent(np); + if (parent_np == NULL) return; - virq = irq_of_parse_and_map(np, 0); - of_node_put(np); + virq = irq_of_parse_and_map(parent_np, 0); + of_node_put(parent_np); } if (!virq) return; -- 2.9.5
Re: [PATCH v2] powerpc/boot: pass CONFIG options in a simpler and more robust way
On Thu, Jul 4, 2019 at 9:26 AM Michael Ellerman wrote: > > Masahiro Yamada writes: > > > Commit 5e9dcb6188a4 ("powerpc/boot: Expose Kconfig symbols to wrapper") > > was wrong, but commit e41b93a6be57 ("powerpc/boot: Fix build failures > > with -j 1") was also wrong. > > > > The correct dependency is: > > > > $(obj)/serial.o: $(obj)/autoconf.h > > > > However, I do not see the reason why we need to copy autoconf.h to > > arch/power/boot/. Nor do I see consistency in the way of passing > > CONFIG options. > > > > decompress.c references CONFIG_KERNEL_GZIP and CONFIG_KERNEL_XZ, which > > are passed via the command line. > > > > serial.c includes autoconf.h to reference a couple of CONFIG options, > > but this is fragile because we often forget to include "autoconf.h" > > from source files. > > > > In fact, it is already broken. > > > > ppc_asm.h references CONFIG_PPC_8xx, but utils.S is not given any way > > to access CONFIG options. So, CONFIG_PPC_8xx is never defined here. > > > > Pass $(LINUXINCLUDE) to make sure CONFIG options are accessible from > > all .c and .S files in arch/powerpc/boot/. > > This breaks our skiroot_defconfig, I don't know why yet: > > In file included from > /kisskb/src/arch/powerpc/boot/../../../lib/decompress_unxz.c:236:0, >from /kisskb/src/arch/powerpc/boot/decompress.c:42: > /kisskb/src/arch/powerpc/boot/../../../lib/xz/xz_dec_bcj.c: In function > 'bcj_powerpc': > /kisskb/src/arch/powerpc/boot/../../../lib/xz/xz_dec_bcj.c:166:11: warning: > implicit declaration of function 'get_unaligned_be32' > [-Wimplicit-function-declaration] > instr = get_unaligned_be32(buf + i); OK, now I see the cause of the error. I will insert a new patch in v3. Thanks. > > http://kisskb.ellerman.id.au/kisskb/buildresult/13862914/ > > cheers -- Best Regards Masahiro Yamada
[PATCH v3 1/2] powerpc/boot: add {get, put}_unaligned_be32 to xz_config.h
The next commit will make the way of passing CONFIG options more robust. Unfortunately, it would uncover another hidden issue; without this commit, skiroot_defconfig would be broken like this: | WRAParch/powerpc/boot/zImage.pseries | arch/powerpc/boot/wrapper.a(decompress.o): In function `bcj_powerpc.isra.10': | decompress.c:(.text+0x720): undefined reference to `get_unaligned_be32' | decompress.c:(.text+0x7a8): undefined reference to `put_unaligned_be32' | make[1]: *** [arch/powerpc/boot/Makefile;383: arch/powerpc/boot/zImage.pseries] Error 1 | make: *** [arch/powerpc/Makefile;295: zImage] Error 2 skiroot_defconfig is the only defconfig that enables CONFIG_KERNEL_XZ for ppc, which has never been correctly built before. I figured out the root cause in lib/decompress_unxz.c: | #ifdef CONFIG_PPC | # define XZ_DEC_POWERPC | #endif CONFIG_PPC is undefined here in the ppc bootwrapper because autoconf.h is not included except by arch/powerpc/boot/serial.c XZ_DEC_POWERPC is not defined, therefore, bcj_powerpc() is not compiled for the bootwrapper. With the next commit passing CONFIG_PPC correctly, we would realize that {get,put}_unaligned_be32 was missing. Unlike the other decompressors, the ppc bootwrapper duplicates all the necessary helpers in arch/powerpc/boot/. The other architectures define __KERNEL__ and pull in helpers for building the decompressors. If ppc bootwrapper had defined __KERNEL__, lib/xz/xz_private.h would have included : | #ifdef __KERNEL__ | # include | # include | # include However, doing so would cause tons of definition conflicts since the bootwrapper has duplicated everything. I just added copies of {get,put}_unaligned_be32, following the bootwrapper coding convention. Signed-off-by: Masahiro Yamada --- Changes in v3: - New patch to fix the potential issue of skiroot_defconfig Changes in v2: None arch/powerpc/boot/xz_config.h | 20 1 file changed, 20 insertions(+) diff --git a/arch/powerpc/boot/xz_config.h b/arch/powerpc/boot/xz_config.h index e22e5b3770dd..ebfadd39e192 100644 --- a/arch/powerpc/boot/xz_config.h +++ b/arch/powerpc/boot/xz_config.h @@ -20,10 +20,30 @@ static inline uint32_t swab32p(void *p) #ifdef __LITTLE_ENDIAN__ #define get_le32(p) (*((uint32_t *) (p))) +#define cpu_to_be32(x) swab32(x) +static inline u32 be32_to_cpup(const u32 *p) +{ + return swab32p((u32 *)p); +} #else #define get_le32(p) swab32p(p) +#define cpu_to_be32(x) (x) +static inline u32 be32_to_cpup(const u32 *p) +{ + return *p; +} #endif +static inline uint32_t get_unaligned_be32(const void *p) +{ + return be32_to_cpup(p); +} + +static inline void put_unaligned_be32(u32 val, void *p) +{ + *((u32 *)p) = cpu_to_be32(val); +} + #define memeq(a, b, size) (memcmp(a, b, size) == 0) #define memzero(buf, size) memset(buf, 0, size) -- 2.17.1
[PATCH v3 2/2] powerpc/boot: pass CONFIG options in a simpler and more robust way
Commit 5e9dcb6188a4 ("powerpc/boot: Expose Kconfig symbols to wrapper") was wrong, but commit e41b93a6be57 ("powerpc/boot: Fix build failures with -j 1") was also wrong. The correct dependency is: $(obj)/serial.o: $(obj)/autoconf.h However, I do not see the reason why we need to copy autoconf.h to arch/power/boot/. Nor do I see consistency in the way of passing CONFIG options. decompress.c references CONFIG_KERNEL_GZIP and CONFIG_KERNEL_XZ, which are passed via the command line. serial.c includes autoconf.h to reference a couple of CONFIG options, but this is fragile because we often forget to include "autoconf.h" from source files. In fact, it is already broken. ppc_asm.h references CONFIG_PPC_8xx, but utils.S is not given any way to access CONFIG options. So, CONFIG_PPC_8xx is never defined here. Pass $(LINUXINCLUDE) to make sure CONFIG options are accessible from all .c and .S files in arch/powerpc/boot/. I also removed the -traditional flag to make include/linux/kconfig.h work. This flag makes the preprocessor imitate the behavior of the pre-standard C compiler, but I do not understand why it is necessary. Signed-off-by: Masahiro Yamada --- Changes in v3: None Changes in v2: - reword commit log arch/powerpc/boot/.gitignore | 2 -- arch/powerpc/boot/Makefile | 14 +++--- arch/powerpc/boot/serial.c | 1 - 3 files changed, 3 insertions(+), 14 deletions(-) diff --git a/arch/powerpc/boot/.gitignore b/arch/powerpc/boot/.gitignore index 32034a0cc554..6610665fcf5e 100644 --- a/arch/powerpc/boot/.gitignore +++ b/arch/powerpc/boot/.gitignore @@ -44,5 +44,3 @@ fdt_sw.c fdt_wip.c libfdt.h libfdt_internal.h -autoconf.h - diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile index 73d1f3562978..b8a82be2af2a 100644 --- a/arch/powerpc/boot/Makefile +++ b/arch/powerpc/boot/Makefile @@ -20,9 +20,6 @@ all: $(obj)/zImage -compress-$(CONFIG_KERNEL_GZIP) := CONFIG_KERNEL_GZIP -compress-$(CONFIG_KERNEL_XZ) := CONFIG_KERNEL_XZ - ifdef CROSS32_COMPILE BOOTCC := $(CROSS32_COMPILE)gcc BOOTAR := $(CROSS32_COMPILE)ar @@ -34,7 +31,7 @@ endif BOOTCFLAGS:= -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \ -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \ --D$(compress-y) +$(LINUXINCLUDE) ifdef CONFIG_PPC64_BOOT_WRAPPER BOOTCFLAGS += -m64 @@ -51,7 +48,7 @@ BOOTCFLAGS+= -mlittle-endian BOOTCFLAGS += $(call cc-option,-mabi=elfv2) endif -BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -traditional -nostdinc +BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -nostdinc BOOTARFLAGS:= -cr$(KBUILD_ARFLAGS) @@ -202,14 +199,9 @@ $(obj)/empty.c: $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds : $(obj)/%: $(srctree)/$(src)/%.S $(Q)cp $< $@ -$(srctree)/$(src)/serial.c: $(obj)/autoconf.h - -$(obj)/autoconf.h: $(obj)/%: $(objtree)/include/generated/% - $(Q)cp $< $@ - clean-files := $(zlib-) $(zlibheader-) $(zliblinuxheader-) \ $(zlib-decomp-) $(libfdt) $(libfdtheader) \ - autoconf.h empty.c zImage.coff.lds zImage.ps3.lds zImage.lds + empty.c zImage.coff.lds zImage.ps3.lds zImage.lds quiet_cmd_bootcc = BOOTCC $@ cmd_bootcc = $(BOOTCC) -Wp,-MD,$(depfile) $(BOOTCFLAGS) -c -o $@ $< diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c index b0491b8c0199..9457863147f9 100644 --- a/arch/powerpc/boot/serial.c +++ b/arch/powerpc/boot/serial.c @@ -18,7 +18,6 @@ #include "stdio.h" #include "io.h" #include "ops.h" -#include "autoconf.h" static int serial_open(void) { -- 2.17.1
Re: [PATCH] mm/kprobes: Add generic kprobe_fault_handler() fallback definition
Hi Anshuman, On Fri, 5 Jul 2019 11:00:29 +0530 Anshuman Khandual wrote: > Architectures like parisc enable CONFIG_KROBES without having a definition > for kprobe_fault_handler() which results in a build failure. Hmm, as far as I can see, kprobe_fault_handler() is closed inside each arch specific code. The reason why include/linux/kprobes.h defines dummy inline function is only for !CONFIG_KPROBES case. > Arch needs to > provide kprobe_fault_handler() as it is platform specific and cannot have > a generic working alternative. But in the event when platform lacks such a > definition there needs to be a fallback. Wait, indeed that each arch need to implement it, but that is for calling kprobe->fault_handler() as user expected. Hmm, why not fixing those architecture implementations? > This adds a stub kprobe_fault_handler() definition which not only prevents > a build failure but also makes sure that kprobe_page_fault() if called will > always return negative in absence of a sane platform specific alternative. I don't like introducing this complicated macro only for avoiding (not fixing) build error. To fix that, kprobes on parisc should implement kprobe_fault_handler correctly (and call kprobe->fault_handler). BTW, even if you need such generic stub, please use a weak function instead of macros for every arch headers. > While here wrap kprobe_page_fault() in CONFIG_KPROBES. This enables stud > definitions for generic kporbe_fault_handler() and kprobes_built_in() can > just be dropped. Only on x86 it needs to be added back locally as it gets > used in a !CONFIG_KPROBES function do_general_protection(). If you want to remove kprobes_built_in(), you should replace it with IS_ENABLED(CONFIG_KPROBES), instead of this... Thank you, > > Cc: Vineet Gupta > Cc: Russell King > Cc: Catalin Marinas > Cc: Will Deacon > Cc: Tony Luck > Cc: Fenghua Yu > Cc: Ralf Baechle > Cc: Paul Burton > Cc: James Hogan > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Cc: Michael Ellerman > Cc: Heiko Carstens > Cc: Vasily Gorbik > Cc: Christian Borntraeger > Cc: Yoshinori Sato > Cc: Rich Felker > Cc: "David S. Miller" > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: Borislav Petkov > Cc: "H. Peter Anvin" > Cc: "Naveen N. Rao" > Cc: Anil S Keshavamurthy > Cc: Masami Hiramatsu > Cc: Allison Randal > Cc: Greg Kroah-Hartman > Cc: Enrico Weigelt > Cc: Richard Fontana > Cc: Kate Stewart > Cc: Mark Rutland > Cc: Andrew Morton > Cc: Guenter Roeck > Cc: x...@kernel.org > Cc: linux-snps-...@lists.infradead.org > Cc: linux-ker...@vger.kernel.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-i...@vger.kernel.org > Cc: linux-m...@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Cc: linux-s...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: sparcli...@vger.kernel.org > > Signed-off-by: Anshuman Khandual > --- > arch/arc/include/asm/kprobes.h | 1 + > arch/arm/include/asm/kprobes.h | 1 + > arch/arm64/include/asm/kprobes.h | 1 + > arch/ia64/include/asm/kprobes.h| 1 + > arch/mips/include/asm/kprobes.h| 1 + > arch/powerpc/include/asm/kprobes.h | 1 + > arch/s390/include/asm/kprobes.h| 1 + > arch/sh/include/asm/kprobes.h | 1 + > arch/sparc/include/asm/kprobes.h | 1 + > arch/x86/include/asm/kprobes.h | 6 ++ > include/linux/kprobes.h| 32 ++ > 11 files changed, 34 insertions(+), 13 deletions(-) > > diff --git a/arch/arc/include/asm/kprobes.h b/arch/arc/include/asm/kprobes.h > index 2134721dce44..ee8efe256675 100644 > --- a/arch/arc/include/asm/kprobes.h > +++ b/arch/arc/include/asm/kprobes.h > @@ -45,6 +45,7 @@ struct kprobe_ctlblk { > struct prev_kprobe prev_kprobe; > }; > > +#define kprobe_fault_handler kprobe_fault_handler > int kprobe_fault_handler(struct pt_regs *regs, unsigned long cause); > void kretprobe_trampoline(void); > void trap_is_kprobe(unsigned long address, struct pt_regs *regs); > diff --git a/arch/arm/include/asm/kprobes.h b/arch/arm/include/asm/kprobes.h > index 213607a1f45c..660f877b989f 100644 > --- a/arch/arm/include/asm/kprobes.h > +++ b/arch/arm/include/asm/kprobes.h > @@ -38,6 +38,7 @@ struct kprobe_ctlblk { > struct prev_kprobe prev_kprobe; > }; > > +#define kprobe_fault_handler kprobe_fault_handler > void arch_remove_kprobe(struct kprobe *); > int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr); > int kprobe_exceptions_notify(struct notifier_block *self, > diff --git a/arch/arm64/include/asm/kprobes.h > b/arch/arm64/include/asm/kprobes.h > index 97e511d645a2..667773f75616 100644 > --- a/arch/arm64/include/asm/kprobes.h > +++ b/arch/arm64/include/asm/kprobes.h > @@ -42,6 +42,7 @@ struct kprobe_ctlblk { > struct kprobe_step_ctx ss_ctx; > }; > > +#define kprobe_fault_handler kprobe_fault_handler > void arch_remove_kprobe(struct kprobe *); > int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr); > int kprobe_exc
Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
On Wed, 2019-07-03 at 23:32 -0400, Nayna Jain wrote: > The nr_allocated_banks and allocated banks are initialized as part of > tpm_chip_register. Currently, this is done as part of auto startup > function. However, some drivers, like the ibm vtpm driver, do not run > auto startup during initialization. This results in uninitialized memory > issue and causes a kernel panic during boot. > > This patch moves the pcr allocation outside the auto startup function > into tpm_chip_register. This ensures that allocated banks are initialized > in any case. > > Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with > PCR read") > Signed-off-by: Nayna Jain Please add Reported-by: Michal Suchanek It is missing. Michal is there a chance you could try this out once Nayna send a new version? > --- > drivers/char/tpm/tpm-chip.c | 37 + > drivers/char/tpm/tpm.h | 1 + > drivers/char/tpm/tpm1-cmd.c | 12 > drivers/char/tpm/tpm2-cmd.c | 6 +- > 4 files changed, 39 insertions(+), 17 deletions(-) > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > index 8804c9e916fd..958508bb8379 100644 > --- a/drivers/char/tpm/tpm-chip.c > +++ b/drivers/char/tpm/tpm-chip.c > @@ -550,6 +550,39 @@ static int tpm_add_hwrng(struct tpm_chip *chip) > return hwrng_register(&chip->hwrng); > } > > +/* > + * tpm_pcr_allocation() - initializes the chip allocated banks for PCRs > + */ > +static int tpm_pcr_allocation(struct tpm_chip *chip) Why that name and not tpm_get_pcr_allocation()? Do not get why "get_" has been dropped. Please add it back. Would be senseful to create tpm1_get_pcr_allocation() to tpm1-cmd.c now that a new function needs to be introduced anyway. Please do it for that for TPM 1.x part. /Jarkko
Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
On Fri, 05 Jul 2019 13:42:18 +0300 Jarkko Sakkinen wrote: > On Wed, 2019-07-03 at 23:32 -0400, Nayna Jain wrote: > > The nr_allocated_banks and allocated banks are initialized as part of > > tpm_chip_register. Currently, this is done as part of auto startup > > function. However, some drivers, like the ibm vtpm driver, do not run > > auto startup during initialization. This results in uninitialized memory > > issue and causes a kernel panic during boot. > > > > This patch moves the pcr allocation outside the auto startup function > > into tpm_chip_register. This ensures that allocated banks are initialized > > in any case. > > > > Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with > > PCR read") > > Signed-off-by: Nayna Jain > > Please add > > Reported-by: Michal Suchanek > > It is missing. Michal is there a chance you could try this out once > Nayna send a new version? Should be easy to test. Without the patch the machine crashes on boot so it is quite obvious case. I have a few VMs with the vTPM available. Thanks Michal
Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
On Fri, 2019-07-05 at 13:42 +0300, Jarkko Sakkinen wrote: > On Wed, 2019-07-03 at 23:32 -0400, Nayna Jain wrote: > > The nr_allocated_banks and allocated banks are initialized as part of > > tpm_chip_register. Currently, this is done as part of auto startup > > function. However, some drivers, like the ibm vtpm driver, do not run > > auto startup during initialization. This results in uninitialized memory > > issue and causes a kernel panic during boot. > > > > This patch moves the pcr allocation outside the auto startup function > > into tpm_chip_register. This ensures that allocated banks are initialized > > in any case. > > > > Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with > > PCR read") > > Signed-off-by: Nayna Jain > > Please add > > Reported-by: Michal Suchanek > > It is missing. Michal is there a chance you could try this out once > Nayna send a new version? Hey, I saw Michal's tested-by. I can do that minor reorg cosmetic bits myeslf and add Micha's tag. Some issue with the network but I'll push a commit soonish. /Jarkko
Re: [PATCH v12 01/11] MODSIGN: Export module signature definitions
Hi Thiago, On Thu, 04 Jul 2019 15:57:34 -0300 Thiago Jung Bauermann wrote: > Hello Philipp, > > Philipp Rudo writes: > > > Hi Thiago, > > > > > > On Thu, 04 Jul 2019 03:42:57 -0300 > > Thiago Jung Bauermann wrote: > > > >> Jessica Yu writes: > >> > >> > +++ Thiago Jung Bauermann [27/06/19 23:19 -0300]: > >> >>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 > >> >>Reviewed-by: Mimi Zohar > >> >>Cc: Jessica Yu > >> >>--- > >> >> include/linux/module.h | 3 -- > >> >> include/linux/module_signature.h | 44 + > >> >> init/Kconfig | 6 +++- > >> >> kernel/Makefile | 1 + > >> >> kernel/module.c | 1 + > >> >> kernel/module_signature.c| 46 ++ > >> >> kernel/module_signing.c | 56 +--- > >> >> scripts/Makefile | 2 +- > >> >> 8 files changed, 106 insertions(+), 53 deletions(-) > >> >> > >> >>diff --git a/include/linux/module.h b/include/linux/module.h > >> >>index 188998d3dca9..aa56f531cf1e 100644 > >> >>--- a/include/linux/module.h > >> >>+++ b/include/linux/module.h > >> >>@@ -25,9 +25,6 @@ > >> >> #include > >> >> #include > >> >> > >> >>-/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */ > >> >>-#define MODULE_SIG_STRING "~Module signature appended~\n" > >> >>- > >> > > >> > Hi Thiago, apologies for the delay. > >> > >> Hello Jessica, thanks for reviewing the patch! > >> > >> > It looks like arch/s390/kernel/machine_kexec_file.c also relies on > >> > MODULE_SIG_STRING being defined, so module_signature.h will need to be > >> > included there too, otherwise we'll run into a compilation error. > >> > >> Indeed. Thanks for spotting that. The patch below fixes it. It's > >> identical to the previous version except for the changes in > >> arch/s390/kernel/machine_kexec_file.c and their description in the > >> commit message. I'm also copying some s390 people in this email. > > > > to me the s390 part looks good but for one minor nit. > > Thanks for the prompt review! > > > In arch/s390/Kconfig KEXEC_VERIFY_SIG currently depends on > > SYSTEM_DATA_VERIFICATION. I'd prefer when you update this to the new > > MODULE_SIG_FORMAT. It shouldn't make any difference right now, as we don't > > use mod_check_sig in our code path. But it could cause problems in the > > future, > > when more code might be shared. > > Makes sense. Here is the updated patch with the Kconfig change. > The patch looks good now. Thanks a lot PHilipp
Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
On 7/3/19 11:32 PM, Nayna Jain wrote: The nr_allocated_banks and allocated banks are initialized as part of tpm_chip_register. Currently, this is done as part of auto startup function. However, some drivers, like the ibm vtpm driver, do not run auto startup during initialization. This results in uninitialized memory issue and causes a kernel panic during boot. This patch moves the pcr allocation outside the auto startup function into tpm_chip_register. This ensures that allocated banks are initialized in any case. Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with PCR read") Signed-off-by: Nayna Jain --- drivers/char/tpm/tpm-chip.c | 37 + drivers/char/tpm/tpm.h | 1 + drivers/char/tpm/tpm1-cmd.c | 12 drivers/char/tpm/tpm2-cmd.c | 6 +- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index 8804c9e916fd..958508bb8379 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -550,6 +550,39 @@ static int tpm_add_hwrng(struct tpm_chip *chip) return hwrng_register(&chip->hwrng); } +/* + * tpm_pcr_allocation() - initializes the chip allocated banks for PCRs + */ +static int tpm_pcr_allocation(struct tpm_chip *chip) +{ + int rc = 0; + + if (chip->flags & TPM_CHIP_FLAG_TPM2) { + rc = tpm2_get_pcr_allocation(chip); + if (rc) + goto out; + } + + /* Initialize TPM 1.2 */ + chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks), + GFP_KERNEL); + if (!chip->allocated_banks) { + rc = -ENOMEM; + goto out; + } + + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1; + chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1]; + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1; + chip->nr_allocated_banks = 1; + + return 0; +out: + if (rc < 0) + rc = -ENODEV; The old code where you lifted this from said: out: if (rc > 0) rc = -ENODEV; return rc; It would not overwrite -ENOMEM with -ENODEV but yours does. I think the correct fix would be to use: if (rc > 0) rc = -ENODEV; + return rc; +} + /* * tpm_chip_register() - create a character device for the TPM chip * @chip: TPM chip to use. @@ -573,6 +606,10 @@ int tpm_chip_register(struct tpm_chip *chip) if (rc) return rc; Above this is tpm_chip_stop(chip) because (afaik) none of the following function calls in tpm_chip_register() needed the TPM, but now with tpm_pcr_allocation() you will need to send a command to the TPM. So I would say you should move the tpm_chip_stop() into the error branch visible above and also after the tpm_pcr_allocation(). + rc = tpm_pcr_allocation(chip); tpm_chip_stop(chip); + if (rc) + return rc; + tpm_sysfs_add_device(chip); rc = tpm_bios_log_setup(chip); Stefan
Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
On 07/05/2019 10:13 AM, Stefan Berger wrote: On 7/3/19 11:32 PM, Nayna Jain wrote: The nr_allocated_banks and allocated banks are initialized as part of tpm_chip_register. Currently, this is done as part of auto startup function. However, some drivers, like the ibm vtpm driver, do not run auto startup during initialization. This results in uninitialized memory issue and causes a kernel panic during boot. This patch moves the pcr allocation outside the auto startup function into tpm_chip_register. This ensures that allocated banks are initialized in any case. Fixes: 879b589210a9 ("tpm: retrieve digest size of unknown algorithms with PCR read") Signed-off-by: Nayna Jain --- drivers/char/tpm/tpm-chip.c | 37 + drivers/char/tpm/tpm.h | 1 + drivers/char/tpm/tpm1-cmd.c | 12 drivers/char/tpm/tpm2-cmd.c | 6 +- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index 8804c9e916fd..958508bb8379 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -550,6 +550,39 @@ static int tpm_add_hwrng(struct tpm_chip *chip) return hwrng_register(&chip->hwrng); } +/* + * tpm_pcr_allocation() - initializes the chip allocated banks for PCRs + */ +static int tpm_pcr_allocation(struct tpm_chip *chip) +{ + int rc = 0; + + if (chip->flags & TPM_CHIP_FLAG_TPM2) { + rc = tpm2_get_pcr_allocation(chip); + if (rc) + goto out; + } + + /* Initialize TPM 1.2 */ + chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks), + GFP_KERNEL); + if (!chip->allocated_banks) { + rc = -ENOMEM; + goto out; + } + + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1; + chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1]; + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1; + chip->nr_allocated_banks = 1; + + return 0; +out: + if (rc < 0) + rc = -ENODEV; The old code where you lifted this from said: out: if (rc > 0) rc = -ENODEV; return rc; It would not overwrite -ENOMEM with -ENODEV but yours does. I think the correct fix would be to use: if (rc > 0) rc = -ENODEV; Yes. I think I misread it. Thanks Stefan. Will fix this.. + return rc; +} + /* * tpm_chip_register() - create a character device for the TPM chip * @chip: TPM chip to use. @@ -573,6 +606,10 @@ int tpm_chip_register(struct tpm_chip *chip) if (rc) return rc; Above this is tpm_chip_stop(chip) because (afaik) none of the following function calls in tpm_chip_register() needed the TPM, but now with tpm_pcr_allocation() you will need to send a command to the TPM. So I would say you should move the tpm_chip_stop() into the error branch visible above and also after the tpm_pcr_allocation(). + rc = tpm_pcr_allocation(chip); tpm_chip_stop(chip); I am not sure of the purpose of tpm_stop_chip(), so I have left it as it is. Jarkko, what do you think about the change ? Thanks & Regards, - Nayna
Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
On Fri, 2019-07-05 at 11:32 -0400, Nayna wrote: > I am not sure of the purpose of tpm_stop_chip(), so I have left it as it > is. Jarkko, what do you think about the change ? Stefan right. Your does not work, or will randomly work or not work depending on the chip. You need to turn the TPM on with tpm_chip_start() and turn it off with tpm_chip_stop() once you are done. This is done in tpm_chip_register() before calling tpm_auto_startup(). TPM power management was once in tpm_transmit() but not anymore after my patch set that removed nested tpm_transmit() calls. While you're on it please take into account my earlier feedback. Also, short summary could be "tpm: tpm_ibm_vtpm: Fix unallocated banks" Some oddballs in your patch that I have to ask. if (chip->flags & TPM_CHIP_FLAG_TPM2) { rc = tpm2_get_pcr_allocation(chip); if (rc) goto out; } chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks), GFP_KERNEL); if (!chip->allocated_banks) { rc = -ENOMEM; goto out; } Why you don't return on site and instead jump somewhere? Also the 2nd line for kcalloc() is misaligned. out: if (rc < 0) rc = -ENODEV; This will cause a new regression i.e. you let TPM error codes through. To summarize this patch fixes one regression and introduces two completely new ones... /Jarkko`
Re: [PATCH] tpm: fixes uninitialized allocated banks for IBM vtpm driver
On Fri, 2019-07-05 at 20:50 +0300, Jarkko Sakkinen wrote: > To summarize this patch fixes one regression and introduces two > completely new ones... Anyway, take the time and update it. The principle is right anyway. I'll merge it once it is in a better shape... /Jarkko
Re: [PATCH] i2c: remove casting dma_alloc
On Sun, Jun 23, 2019 at 11:13:53PM +0200, Vasyl Gomonovych wrote: > From: Vasyl > > Generated by: alloc_cast.cocci > > Signed-off-by: Vasyl Applied to for-next, thanks! I fixed the missing full name for you, but please fix your setup. signature.asc Description: PGP signature
[v3 1/7] powerpc/mce: Make machine_check_ue_event() static
From: Reza Arbab The function doesn't get used outside this file, so make it static. Signed-off-by: Reza Arbab Signed-off-by: Santosh Sivaraj --- arch/powerpc/kernel/mce.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c index b18df633eae9..e78c4f18ea0a 100644 --- a/arch/powerpc/kernel/mce.c +++ b/arch/powerpc/kernel/mce.c @@ -33,7 +33,7 @@ static DEFINE_PER_CPU(struct machine_check_event[MAX_MC_EVT], mce_ue_event_queue); static void machine_check_process_queued_event(struct irq_work *work); -void machine_check_ue_event(struct machine_check_event *evt); +static void machine_check_ue_event(struct machine_check_event *evt); static void machine_process_ue_event(struct work_struct *work); static struct irq_work mce_event_process_work = { @@ -203,7 +203,7 @@ void release_mce_event(void) /* * Queue up the MCE event which then can be handled later. */ -void machine_check_ue_event(struct machine_check_event *evt) +static void machine_check_ue_event(struct machine_check_event *evt) { int index; -- 2.20.1
[v3 0/7] powerpc: implement machine check safe memcpy
During a memcpy from a pmem device, if a machine check exception is generated we end up in a panic. In case of fsdax read, this should only result in a -EIO. Avoid MCE by implementing memcpy_mcsafe. Before this patch series: ``` bash-4.4# mount -o dax /dev/pmem0 /mnt/pmem/ [ 7621.714094] Disabling lock debugging due to kernel taint [ 7621.714099] MCE: CPU0: machine check (Severe) Host UE Load/Store [Not recovered] [ 7621.714104] MCE: CPU0: NIP: [c0088978] memcpy_power7+0x418/0x7e0 [ 7621.714107] MCE: CPU0: Hardware error [ 7621.714112] opal: Hardware platform error: Unrecoverable Machine Check exception [ 7621.714118] CPU: 0 PID: 1368 Comm: mount Tainted: G M 5.2.0-rc5-00239-g241e39004581 #50 [ 7621.714123] NIP: c0088978 LR: c08e16f8 CTR: 01de [ 7621.714129] REGS: c000fffbfd70 TRAP: 0200 Tainted: G M (5.2.0-rc5-00239-g241e39004581) [ 7621.714131] MSR: 92209033 CR: 24428840 XER: 0004 [ 7621.714160] CFAR: c00889a8 DAR: deadbeefdeadbeef DSISR: 8000 IRQMASK: 0 [ 7621.714171] GPR00: 0e00 c000f0b8b1e0 c12cf100 c000ed8e1100 [ 7621.714186] GPR04: c2001100 0001 0200 03fff1272000 [ 7621.714201] GPR08: 8000 0010 0020 0030 [ 7621.714216] GPR12: 0040 7fffb8c6d390 0050 0060 [ 7621.714232] GPR16: 0070 0001 c000f0b8b960 [ 7621.714247] GPR20: 0001 c000f0b8b940 0001 0001 [ 7621.714262] GPR24: c1382560 c00c003b6380 c00c003b6380 0001 [ 7621.714277] GPR28: 0001 c200 0001 [ 7621.714294] NIP [c0088978] memcpy_power7+0x418/0x7e0 [ 7621.714298] LR [c08e16f8] pmem_do_bvec+0xf8/0x430 ... ... ``` After this patch series: ``` bash-4.4# mount -o dax /dev/pmem0 /mnt/pmem/ [25302.883978] Buffer I/O error on dev pmem0, logical block 0, async page read [25303.020816] EXT4-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk [25303.021236] EXT4-fs (pmem0): Can't read superblock on 2nd try [25303.152515] EXT4-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk [25303.284031] EXT4-fs (pmem0): DAX enabled. Warning: EXPERIMENTAL, use at your own risk [25304.084100] UDF-fs: bad mount option "dax" or missing value mount: /mnt/pmem: wrong fs type, bad option, bad superblock on /dev/pmem0, missing codepage or helper program, or other error. ``` MCE is injected on a pmem address using mambo. The last patch which adds a nop is only for testing on mambo, where r13 is not restored upon hitting vector 200. The memcpy code can be optimised by adding VMX optimizations and GAS macros can be used to enable code reusablity, which I will send as another series. --- Change-log: v3: * Drop patch which enables DR/IR for external modules * Drop notifier call chain, we don't want to do that in real mode * Return remaining bytes from memcpy_mcsafe correctly * We no longer restore r13 for simulator tests, rather use a nop at vector 0x200 [workaround for simulator; not to be merged] v2: * Don't set RI bit explicitly [mahesh] * Re-ordered series to get r13 workaround as the last patch --- Balbir Singh (2): powerpc/mce: Bug fixes for MCE handling in kernel space powerpc/memcpy: Add memcpy_mcsafe for pmem Reza Arbab (2): powerpc/mce: Make machine_check_ue_event() static powerpc/64s: save r13 in MCE handler (simulator workaroud) Santosh Sivaraj (3): powerpc/mce: Handle UE event for memcpy_mcsafe powerpc/memcpy_mcsafe: return remaining bytes powerpc: add machine check safe copy_to_user arch/powerpc/Kconfig | 1 + arch/powerpc/include/asm/mce.h | 7 +- arch/powerpc/include/asm/string.h| 2 + arch/powerpc/include/asm/uaccess.h | 12 ++ arch/powerpc/kernel/exceptions-64s.S | 1 + arch/powerpc/kernel/mce.c| 11 +- arch/powerpc/kernel/mce_power.c | 41 +++-- arch/powerpc/lib/Makefile| 2 +- arch/powerpc/lib/memcpy_mcsafe_64.S | 239 +++ arch/powerpc/platforms/pseries/ras.c | 6 +- 10 files changed, 302 insertions(+), 20 deletions(-) create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S -- 2.20.1
[v3 2/7] powerpc/mce: Bug fixes for MCE handling in kernel space
From: Balbir Singh The code currently assumes PAGE_SHIFT as the shift value of the pfn, this works correctly (mostly) for user space pages, but the correct thing to do is 1. Extract the shift value returned via the pte-walk API's 2. Use the shift value to access the instruction address. Note, the final physical address still use PAGE_SHIFT for computation. handle_ierror() is not modified and handle_derror() is modified just for extracting the correct instruction address. This is largely due to __find_linux_pte() returning pfn's shifted by pdshift. The code is much more generic and can handle shift values returned. Fixes: ba41e1e1ccb9 ("powerpc/mce: Hookup derror (load/store) UE errors") Signed-off-by: Balbir Singh [ar...@linux.ibm.com: Fixup pseries_do_memory_failure()] Signed-off-by: Reza Arbab Signed-off-by: Santosh Sivaraj --- arch/powerpc/include/asm/mce.h | 3 ++- arch/powerpc/kernel/mce_power.c | 26 -- arch/powerpc/platforms/pseries/ras.c | 6 -- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h index a4c6a74ad2fb..94888a7025b3 100644 --- a/arch/powerpc/include/asm/mce.h +++ b/arch/powerpc/include/asm/mce.h @@ -209,7 +209,8 @@ extern void release_mce_event(void); extern void machine_check_queue_event(void); extern void machine_check_print_event_info(struct machine_check_event *evt, bool user_mode, bool in_guest); -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr); +unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr, + unsigned int *shift); #ifdef CONFIG_PPC_BOOK3S_64 void flush_and_reload_slb(void); #endif /* CONFIG_PPC_BOOK3S_64 */ diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c index e39536aad30d..04666c0b40a8 100644 --- a/arch/powerpc/kernel/mce_power.c +++ b/arch/powerpc/kernel/mce_power.c @@ -23,7 +23,8 @@ * Convert an address related to an mm to a PFN. NOTE: we are in real * mode, we could potentially race with page table updates. */ -unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr) +unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr, + unsigned int *shift) { pte_t *ptep; unsigned long flags; @@ -36,13 +37,15 @@ unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr) local_irq_save(flags); if (mm == current->mm) - ptep = find_current_mm_pte(mm->pgd, addr, NULL, NULL); + ptep = find_current_mm_pte(mm->pgd, addr, NULL, shift); else - ptep = find_init_mm_pte(addr, NULL); + ptep = find_init_mm_pte(addr, shift); local_irq_restore(flags); if (!ptep || pte_special(*ptep)) return ULONG_MAX; - return pte_pfn(*ptep); + if (!*shift) + *shift = PAGE_SHIFT; + return (pte_val(*ptep) & PTE_RPN_MASK) >> *shift; } /* flush SLBs and reload */ @@ -358,15 +361,16 @@ static int mce_find_instr_ea_and_pfn(struct pt_regs *regs, uint64_t *addr, unsigned long pfn, instr_addr; struct instruction_op op; struct pt_regs tmp = *regs; + unsigned int shift; - pfn = addr_to_pfn(regs, regs->nip); + pfn = addr_to_pfn(regs, regs->nip, &shift); if (pfn != ULONG_MAX) { - instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK); + instr_addr = (pfn << shift) + (regs->nip & ((1 << shift) - 1)); instr = *(unsigned int *)(instr_addr); if (!analyse_instr(&op, &tmp, instr)) { - pfn = addr_to_pfn(regs, op.ea); + pfn = addr_to_pfn(regs, op.ea, &shift); *addr = op.ea; - *phys_addr = (pfn << PAGE_SHIFT); + *phys_addr = (pfn << shift); return 0; } /* @@ -442,12 +446,14 @@ static int mce_handle_ierror(struct pt_regs *regs, if (mce_err->sync_error && table[i].error_type == MCE_ERROR_TYPE_UE) { unsigned long pfn; + unsigned int shift; if (get_paca()->in_mce < MAX_MCE_DEPTH) { - pfn = addr_to_pfn(regs, regs->nip); + pfn = addr_to_pfn(regs, regs->nip, + &shift); if (pfn != ULONG_MAX) { *phys_addr = - (pfn << PAGE_SHIFT); + (pfn << shift); }
[v3 3/7] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Balbir Singh The pmem infrastructure uses memcpy_mcsafe in the pmem layer so as to convert machine check exceptions into a return value on failure in case a machine check exception is encountered during the memcpy. This patch largely borrows from the copyuser_power7 logic and does not add the VMX optimizations, largely to keep the patch simple. If needed those optimizations can be folded in. Signed-off-by: Balbir Singh Acked-by: Nicholas Piggin [ar...@linux.ibm.com: Added symbol export] Signed-off-by: Santosh Sivaraj --- arch/powerpc/include/asm/string.h | 2 + arch/powerpc/lib/Makefile | 2 +- arch/powerpc/lib/memcpy_mcsafe_64.S | 215 3 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h index 9bf6dffb4090..b72692702f35 100644 --- a/arch/powerpc/include/asm/string.h +++ b/arch/powerpc/include/asm/string.h @@ -53,7 +53,9 @@ void *__memmove(void *to, const void *from, __kernel_size_t n); #ifndef CONFIG_KASAN #define __HAVE_ARCH_MEMSET32 #define __HAVE_ARCH_MEMSET64 +#define __HAVE_ARCH_MEMCPY_MCSAFE +extern int memcpy_mcsafe(void *dst, const void *src, __kernel_size_t sz); extern void *__memset16(uint16_t *, uint16_t v, __kernel_size_t); extern void *__memset32(uint32_t *, uint32_t v, __kernel_size_t); extern void *__memset64(uint64_t *, uint64_t v, __kernel_size_t); diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile index c55f9c27bf79..529d6536eb4a 100644 --- a/arch/powerpc/lib/Makefile +++ b/arch/powerpc/lib/Makefile @@ -39,7 +39,7 @@ obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \ memcpy_power7.o obj64-y+= copypage_64.o copyuser_64.o mem_64.o hweight_64.o \ - memcpy_64.o pmem.o + memcpy_64.o pmem.o memcpy_mcsafe_64.o obj64-$(CONFIG_SMP)+= locks.o obj64-$(CONFIG_ALTIVEC)+= vmx-helper.o diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S new file mode 100644 index ..50f865db0338 --- /dev/null +++ b/arch/powerpc/lib/memcpy_mcsafe_64.S @@ -0,0 +1,215 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) IBM Corporation, 2011 + * Derived from copyuser_power7.s by Anton Blanchard + * Author - Balbir Singh + */ +#include +#include +#include + + .macro err1 +100: + EX_TABLE(100b,.Ldo_err1) + .endm + + .macro err2 +200: + EX_TABLE(200b,.Ldo_err2) + .endm + +.Ldo_err2: + ld r22,STK_REG(R22)(r1) + ld r21,STK_REG(R21)(r1) + ld r20,STK_REG(R20)(r1) + ld r19,STK_REG(R19)(r1) + ld r18,STK_REG(R18)(r1) + ld r17,STK_REG(R17)(r1) + ld r16,STK_REG(R16)(r1) + ld r15,STK_REG(R15)(r1) + ld r14,STK_REG(R14)(r1) + addir1,r1,STACKFRAMESIZE +.Ldo_err1: + li r3,-EFAULT + blr + + +_GLOBAL(memcpy_mcsafe) + cmpldi r5,16 + blt .Lshort_copy + +.Lcopy: + /* Get the source 8B aligned */ + neg r6,r4 + mtocrf 0x01,r6 + clrldi r6,r6,(64-3) + + bf cr7*4+3,1f +err1; lbz r0,0(r4) + addir4,r4,1 +err1; stb r0,0(r3) + addir3,r3,1 + +1: bf cr7*4+2,2f +err1; lhz r0,0(r4) + addir4,r4,2 +err1; sth r0,0(r3) + addir3,r3,2 + +2: bf cr7*4+1,3f +err1; lwz r0,0(r4) + addir4,r4,4 +err1; stw r0,0(r3) + addir3,r3,4 + +3: sub r5,r5,r6 + cmpldi r5,128 + blt 5f + + mflrr0 + stdur1,-STACKFRAMESIZE(r1) + std r14,STK_REG(R14)(r1) + std r15,STK_REG(R15)(r1) + std r16,STK_REG(R16)(r1) + std r17,STK_REG(R17)(r1) + std r18,STK_REG(R18)(r1) + std r19,STK_REG(R19)(r1) + std r20,STK_REG(R20)(r1) + std r21,STK_REG(R21)(r1) + std r22,STK_REG(R22)(r1) + std r0,STACKFRAMESIZE+16(r1) + + srdir6,r5,7 + mtctr r6 + + /* Now do cacheline (128B) sized loads and stores. */ + .align 5 +4: +err2; ld r0,0(r4) +err2; ld r6,8(r4) +err2; ld r7,16(r4) +err2; ld r8,24(r4) +err2; ld r9,32(r4) +err2; ld r10,40(r4) +err2; ld r11,48(r4) +err2; ld r12,56(r4) +err2; ld r14,64(r4) +err2; ld r15,72(r4) +err2; ld r16,80(r4) +err2; ld r17,88(r4) +err2; ld r18,96(r4) +err2; ld r19,104(r4) +err2; ld r20,112(r4) +err2; ld r21,120(r4) + addir4,r4,128 +err2; std r0,0(r3) +err2; std r6,8(r3) +err2; std r7,16(r3) +err2; std r8,24(r3) +err2; std r9,32(r3) +err2; std r10,40(r3) +err2; std r11,48(r3) +err2; std r12,56(r3) +err2; std r14,64(r3) +err2; std r15,72(r3) +err2; std r16,8
[v3 4/7] powerpc/mce: Handle UE event for memcpy_mcsafe
If we take a UE on one of the instructions with a fixup entry, set nip to continue exucution at the fixup entry. Stop processing the event further or print it. Based-on-patch-by: Reza Arbab Cc: Reza Arbab Cc: Mahesh Salgaonkar Signed-off-by: Santosh Sivaraj --- arch/powerpc/include/asm/mce.h | 4 +++- arch/powerpc/kernel/mce.c | 7 ++- arch/powerpc/kernel/mce_power.c | 15 +-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h index 94888a7025b3..f74257eb013b 100644 --- a/arch/powerpc/include/asm/mce.h +++ b/arch/powerpc/include/asm/mce.h @@ -122,7 +122,8 @@ struct machine_check_event { enum MCE_UeErrorType ue_error_type:8; u8 effective_address_provided; u8 physical_address_provided; - u8 reserved_1[5]; + u8 ignore_event; + u8 reserved_1[4]; u64 effective_address; u64 physical_address; u8 reserved_2[8]; @@ -193,6 +194,7 @@ struct mce_error_info { enum MCE_Initiator initiator:8; enum MCE_ErrorClass error_class:8; boolsync_error; + boolignore_event; }; #define MAX_MC_EVT 100 diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c index e78c4f18ea0a..94f2bb307537 100644 --- a/arch/powerpc/kernel/mce.c +++ b/arch/powerpc/kernel/mce.c @@ -144,7 +144,9 @@ void save_mce_event(struct pt_regs *regs, long handled, if (phys_addr != ULONG_MAX) { mce->u.ue_error.physical_address_provided = true; mce->u.ue_error.physical_address = phys_addr; - machine_check_ue_event(mce); + mce->u.ue_error.ignore_event = mce_err->ignore_event; + if (!mce->u.ue_error.ignore_event) + machine_check_ue_event(mce); } } return; @@ -230,6 +232,9 @@ void machine_check_queue_event(void) if (!get_mce_event(&evt, MCE_EVENT_RELEASE)) return; + if (evt.error_type == MCE_ERROR_TYPE_UE && evt.u.ue_error.ignore_event) + return; + index = __this_cpu_inc_return(mce_queue_count) - 1; /* If queue is full, just return for now. */ if (index >= MAX_MC_EVT) { diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c index 04666c0b40a8..db4aa98a23ac 100644 --- a/arch/powerpc/kernel/mce_power.c +++ b/arch/powerpc/kernel/mce_power.c @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -18,6 +19,7 @@ #include #include #include +#include /* * Convert an address related to an mm to a PFN. NOTE: we are in real @@ -565,9 +567,18 @@ static int mce_handle_derror(struct pt_regs *regs, return 0; } -static long mce_handle_ue_error(struct pt_regs *regs) +static long mce_handle_ue_error(struct pt_regs *regs, + struct mce_error_info *mce_err) { long handled = 0; + const struct exception_table_entry *entry; + + entry = search_exception_tables(regs->nip); + if (entry) { + mce_err->ignore_event = true; + regs->nip = extable_fixup(entry); + return 1; + } /* * On specific SCOM read via MMIO we may get a machine check @@ -600,7 +611,7 @@ static long mce_handle_error(struct pt_regs *regs, &phys_addr); if (!handled && mce_err.error_type == MCE_ERROR_TYPE_UE) - handled = mce_handle_ue_error(regs); + handled = mce_handle_ue_error(regs, &mce_err); save_mce_event(regs, handled, &mce_err, regs->nip, addr, phys_addr); -- 2.20.1
[v3 5/7] powerpc/memcpy_mcsafe: return remaining bytes
memcpy_mcsafe currently return -EFAULT on a machine check exception, change it to return the remaining bytes that needs to be copied, so that machine check safe copy_to_user can maintain the same behavior as copy_to_user. Signed-off-by: Santosh Sivaraj --- arch/powerpc/lib/memcpy_mcsafe_64.S | 142 1 file changed, 83 insertions(+), 59 deletions(-) diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S index 50f865db0338..4d8a3d315992 100644 --- a/arch/powerpc/lib/memcpy_mcsafe_64.S +++ b/arch/powerpc/lib/memcpy_mcsafe_64.S @@ -30,11 +30,25 @@ ld r14,STK_REG(R14)(r1) addir1,r1,STACKFRAMESIZE .Ldo_err1: - li r3,-EFAULT + /* Do a byte by byte copy to get the exact remaining size */ + mtctr r7 +100: EX_TABLE(100b, .Ldone) +46: +err1; lbz r0,0(r4) + addir4,r4,1 +err1; stb r0,0(r3) + addir3,r3,1 + bdnz46b + li r3,0 + blr + +.Ldone: + mfctr r3 blr _GLOBAL(memcpy_mcsafe) + mr r7,r5 cmpldi r5,16 blt .Lshort_copy @@ -49,18 +63,21 @@ err1; lbz r0,0(r4) addir4,r4,1 err1; stb r0,0(r3) addir3,r3,1 + subir7,r7,1 1: bf cr7*4+2,2f err1; lhz r0,0(r4) addir4,r4,2 err1; sth r0,0(r3) addir3,r3,2 + subir7,r7,2 2: bf cr7*4+1,3f err1; lwz r0,0(r4) addir4,r4,4 err1; stw r0,0(r3) addir3,r3,4 + subir7,r7,4 3: sub r5,r5,r6 cmpldi r5,128 @@ -87,43 +104,69 @@ err1; stw r0,0(r3) 4: err2; ld r0,0(r4) err2; ld r6,8(r4) -err2; ld r7,16(r4) -err2; ld r8,24(r4) -err2; ld r9,32(r4) -err2; ld r10,40(r4) -err2; ld r11,48(r4) -err2; ld r12,56(r4) -err2; ld r14,64(r4) -err2; ld r15,72(r4) -err2; ld r16,80(r4) -err2; ld r17,88(r4) -err2; ld r18,96(r4) -err2; ld r19,104(r4) -err2; ld r20,112(r4) -err2; ld r21,120(r4) +err2; ld r8,16(r4) +err2; ld r9,24(r4) +err2; ld r10,32(r4) +err2; ld r11,40(r4) +err2; ld r12,48(r4) +err2; ld r14,56(r4) +err2; ld r15,64(r4) +err2; ld r16,72(r4) +err2; ld r17,80(r4) +err2; ld r18,88(r4) +err2; ld r19,96(r4) +err2; ld r20,104(r4) +err2; ld r21,112(r4) +err2; ld r22,120(r4) addir4,r4,128 err2; std r0,0(r3) err2; std r6,8(r3) -err2; std r7,16(r3) -err2; std r8,24(r3) -err2; std r9,32(r3) -err2; std r10,40(r3) -err2; std r11,48(r3) -err2; std r12,56(r3) -err2; std r14,64(r3) -err2; std r15,72(r3) -err2; std r16,80(r3) -err2; std r17,88(r3) -err2; std r18,96(r3) -err2; std r19,104(r3) -err2; std r20,112(r3) -err2; std r21,120(r3) +err2; std r8,16(r3) +err2; std r9,24(r3) +err2; std r10,32(r3) +err2; std r11,40(r3) +err2; std r12,48(r3) +err2; std r14,56(r3) +err2; std r15,64(r3) +err2; std r16,72(r3) +err2; std r17,80(r3) +err2; std r18,88(r3) +err2; std r19,96(r3) +err2; std r20,104(r3) +err2; std r21,112(r3) +err2; std r22,120(r3) addir3,r3,128 + subir7,r7,128 bdnz4b clrldi r5,r5,(64-7) - ld r14,STK_REG(R14)(r1) + /* Up to 127B to go */ +5: srdir6,r5,4 + mtocrf 0x01,r6 + +6: bf cr7*4+1,7f +err2; ld r0,0(r4) +err2; ld r6,8(r4) +err2; ld r8,16(r4) +err2; ld r9,24(r4) +err2; ld r10,32(r4) +err2; ld r11,40(r4) +err2; ld r12,48(r4) +err2; ld r14,56(r4) + addir4,r4,64 +err2; std r0,0(r3) +err2; std r6,8(r3) +err2; std r8,16(r3) +err2; std r9,24(r3) +err2; std r10,32(r3) +err2; std r11,40(r3) +err2; std r12,48(r3) +err2; std r14,56(r3) + addir3,r3,64 + subir7,r7,64 + +7: ld r14,STK_REG(R14)(r1) ld r15,STK_REG(R15)(r1) ld r16,STK_REG(R16)(r1) ld r17,STK_REG(R17)(r1) @@ -134,42 +177,19 @@ err2; std r21,120(r3) ld r22,STK_REG(R22)(r1) addir1,r1,STACKFRAMESIZE - /* Up to 127B to go */ -5: srdir6,r5,4 - mtocrf 0x01,r6 - -6: bf cr7*4+1,7f -err1; ld r0,0(r4) -err1; ld r6,8(r4) -err1; ld r7,16(r4) -err1; ld r8,24(r4) -err1; ld r9,32(r4) -err1; ld r10,40(r4) -err1; ld r11,48(r4) -err1; ld r12,56(r4) - addir4,r4,64 -err1; std r0,0(r3) -err1; std r6,8(r3) -err1; std r7,16(r3) -err1; std r8,24(r3) -err1; std r9,32(r3) -err1; std r10,40(r3) -err1; std r11,48(r3) -err1; std r12,56(r3) - addir3,r3,64 - /* Up to 63B to go */ -7: bf cr7*4+2,8f +
[v3 6/7] powerpc: add machine check safe copy_to_user
Use memcpy_mcsafe() implementation to define copy_to_user_mcsafe() Signed-off-by: Santosh Sivaraj --- arch/powerpc/Kconfig | 1 + arch/powerpc/include/asm/uaccess.h | 12 2 files changed, 13 insertions(+) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 8c1c636308c8..a173b392c272 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -134,6 +134,7 @@ config PPC select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION) select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAS_UACCESS_FLUSHCACHE if PPC64 + select ARCH_HAS_UACCESS_MCSAFE if PPC64 select ARCH_HAS_UBSAN_SANITIZE_ALL select ARCH_HAS_ZONE_DEVICE if PPC_BOOK3S_64 select ARCH_HAVE_NMI_SAFE_CMPXCHG diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h index 76f34346b642..f8fcaab4c5bc 100644 --- a/arch/powerpc/include/asm/uaccess.h +++ b/arch/powerpc/include/asm/uaccess.h @@ -386,6 +386,18 @@ static inline unsigned long raw_copy_to_user(void __user *to, return ret; } +static __always_inline unsigned long __must_check +copy_to_user_mcsafe(void __user *to, const void *from, unsigned long n) +{ + if (likely(check_copy_size(from, n, true))) { + allow_write_to_user(to, n); + n = memcpy_mcsafe(to, from, n); + prevent_write_to_user(to, n); + } + + return n; +} + extern unsigned long __clear_user(void __user *addr, unsigned long size); static inline unsigned long clear_user(void __user *addr, unsigned long size) -- 2.20.1
[v3 7/7] powerpc/64s: save r13 in MCE handler (simulator workaroud)
From: Reza Arbab Testing my memcpy_mcsafe() work in progress with an injected UE, I get an error like this immediately after the function returns: BUG: Unable to handle kernel data access at 0x7fff84dec8f8 Faulting instruction address: 0xc008009c00b0 Oops: Kernel access of bad area, sig: 11 [#1] LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV Modules linked in: mce(O+) vmx_crypto crc32c_vpmsum CPU: 0 PID: 1375 Comm: modprobe Tainted: G O 5.1.0-rc6 #267 NIP: c008009c00b0 LR: c008009c00a8 CTR: c0095f90 REGS: c000ee197790 TRAP: 0300 Tainted: G O (5.1.0-rc6) MSR: 9280b033 CR: 88002826 XER: 0004 CFAR: c0095f8c DAR: 7fff84dec8f8 DSISR: 4000 IRQMASK: 0 GPR00: 6c6c6568 c000ee197a20 c008009c8400 fff2 GPR04: c008009c02e0 0006 c3c834c8 GPR08: 0080 776a6681b7fb5100 c008009c01c8 GPR12: c0095f90 7fff84debc00 4d071440 GPR16: 00010601 c008009e c0c98dd8 c0c98d98 GPR20: c3bba970 c008009c04d0 c008009c0618 c01e5820 GPR24: 0100 0001 c3bba958 GPR28: c008009c02e8 c008009c0318 c008009c02e0 NIP [c008009c00b0] cause_ue+0xa8/0xe8 [mce] LR [c008009c00a8] cause_ue+0xa0/0xe8 [mce] After debugging we see that the first instruction at vector 200 is skipped by the simulator, due to which r13 is not saved. Adding a nop at 0x200 fixes the issue. (This commit is needed for testing this series. This should not be taken into the tree) --- arch/powerpc/kernel/exceptions-64s.S | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S index 73ba246ca11d..8e43abb2a744 100644 --- a/arch/powerpc/kernel/exceptions-64s.S +++ b/arch/powerpc/kernel/exceptions-64s.S @@ -255,6 +255,7 @@ EXC_REAL_BEGIN(machine_check, 0x200, 0x100) * some code path might still want to branch into the original * vector */ + nop SET_SCRATCH0(r13) /* save r13 */ EXCEPTION_PROLOG_0(PACA_EXMC) BEGIN_FTR_SECTION -- 2.20.1
[Bug 203839] Kernel 5.2-rc3 fails to boot on a PowerMac G4 3,6: systemd[1]: Failed to bump fs.file-max, ignoring: invalid argument
https://bugzilla.kernel.org/show_bug.cgi?id=203839 Erhard F. (erhar...@mailbox.org) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |CODE_FIX --- Comment #10 from Erhard F. (erhar...@mailbox.org) --- The fix meanwhile found it's way in kernel 5.2-rc7 which boots just fine. Thanks! -- You are receiving this mail because: You are watching the assignee of the bug.
Re: [v3 1/7] powerpc/mce: Make machine_check_ue_event() static
Santosh Sivaraj's on July 6, 2019 7:26 am: > From: Reza Arbab > > The function doesn't get used outside this file, so make it static. > > Signed-off-by: Reza Arbab > Signed-off-by: Santosh Sivaraj Reviewed-by: Nicholas Piggin
Re: [v3 3/7] powerpc/memcpy: Add memcpy_mcsafe for pmem
Le 05/07/2019 à 23:26, Santosh Sivaraj a écrit : From: Balbir Singh The pmem infrastructure uses memcpy_mcsafe in the pmem layer so as to convert machine check exceptions into a return value on failure in case a machine check exception is encountered during the memcpy. This patch largely borrows from the copyuser_power7 logic and does not add the VMX optimizations, largely to keep the patch simple. If needed those optimizations can be folded in. As this patch largely borrows from copyuser_power7(), could we use gas macro in order to share the source code between the two functions and limit code source duplication ? Christophe Signed-off-by: Balbir Singh Acked-by: Nicholas Piggin [ar...@linux.ibm.com: Added symbol export] Signed-off-by: Santosh Sivaraj --- arch/powerpc/include/asm/string.h | 2 + arch/powerpc/lib/Makefile | 2 +- arch/powerpc/lib/memcpy_mcsafe_64.S | 215 3 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 arch/powerpc/lib/memcpy_mcsafe_64.S diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h index 9bf6dffb4090..b72692702f35 100644 --- a/arch/powerpc/include/asm/string.h +++ b/arch/powerpc/include/asm/string.h @@ -53,7 +53,9 @@ void *__memmove(void *to, const void *from, __kernel_size_t n); #ifndef CONFIG_KASAN #define __HAVE_ARCH_MEMSET32 #define __HAVE_ARCH_MEMSET64 +#define __HAVE_ARCH_MEMCPY_MCSAFE +extern int memcpy_mcsafe(void *dst, const void *src, __kernel_size_t sz); extern void *__memset16(uint16_t *, uint16_t v, __kernel_size_t); extern void *__memset32(uint32_t *, uint32_t v, __kernel_size_t); extern void *__memset64(uint64_t *, uint64_t v, __kernel_size_t); diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile index c55f9c27bf79..529d6536eb4a 100644 --- a/arch/powerpc/lib/Makefile +++ b/arch/powerpc/lib/Makefile @@ -39,7 +39,7 @@ obj-$(CONFIG_PPC_BOOK3S_64) += copyuser_power7.o copypage_power7.o \ memcpy_power7.o obj64-y += copypage_64.o copyuser_64.o mem_64.o hweight_64.o \ - memcpy_64.o pmem.o + memcpy_64.o pmem.o memcpy_mcsafe_64.o obj64-$(CONFIG_SMP) += locks.o obj64-$(CONFIG_ALTIVEC) += vmx-helper.o diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S new file mode 100644 index ..50f865db0338 --- /dev/null +++ b/arch/powerpc/lib/memcpy_mcsafe_64.S @@ -0,0 +1,215 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) IBM Corporation, 2011 + * Derived from copyuser_power7.s by Anton Blanchard + * Author - Balbir Singh + */ +#include +#include +#include + + .macro err1 +100: + EX_TABLE(100b,.Ldo_err1) + .endm + + .macro err2 +200: + EX_TABLE(200b,.Ldo_err2) + .endm + +.Ldo_err2: + ld r22,STK_REG(R22)(r1) + ld r21,STK_REG(R21)(r1) + ld r20,STK_REG(R20)(r1) + ld r19,STK_REG(R19)(r1) + ld r18,STK_REG(R18)(r1) + ld r17,STK_REG(R17)(r1) + ld r16,STK_REG(R16)(r1) + ld r15,STK_REG(R15)(r1) + ld r14,STK_REG(R14)(r1) + addir1,r1,STACKFRAMESIZE +.Ldo_err1: + li r3,-EFAULT + blr + + +_GLOBAL(memcpy_mcsafe) + cmpldi r5,16 + blt .Lshort_copy + +.Lcopy: + /* Get the source 8B aligned */ + neg r6,r4 + mtocrf 0x01,r6 + clrldi r6,r6,(64-3) + + bf cr7*4+3,1f +err1; lbz r0,0(r4) + addir4,r4,1 +err1; stb r0,0(r3) + addir3,r3,1 + +1: bf cr7*4+2,2f +err1; lhz r0,0(r4) + addir4,r4,2 +err1; sth r0,0(r3) + addir3,r3,2 + +2: bf cr7*4+1,3f +err1; lwz r0,0(r4) + addir4,r4,4 +err1; stw r0,0(r3) + addir3,r3,4 + +3: sub r5,r5,r6 + cmpldi r5,128 + blt 5f + + mflrr0 + stdur1,-STACKFRAMESIZE(r1) + std r14,STK_REG(R14)(r1) + std r15,STK_REG(R15)(r1) + std r16,STK_REG(R16)(r1) + std r17,STK_REG(R17)(r1) + std r18,STK_REG(R18)(r1) + std r19,STK_REG(R19)(r1) + std r20,STK_REG(R20)(r1) + std r21,STK_REG(R21)(r1) + std r22,STK_REG(R22)(r1) + std r0,STACKFRAMESIZE+16(r1) + + srdir6,r5,7 + mtctr r6 + + /* Now do cacheline (128B) sized loads and stores. */ + .align 5 +4: +err2; ld r0,0(r4) +err2; ld r6,8(r4) +err2; ld r7,16(r4) +err2; ld r8,24(r4) +err2; ld r9,32(r4) +err2; ld r10,40(r4) +err2; ld r11,48(r4) +err2; ld r12,56(r4) +err2; ld r14,64(r4) +err2; ld r15,72(r4) +err2; ld r16,80(r4) +err2; ld r17,88(r4) +err2; ld r18,96(r4) +err2; ld r19,104(r4) +err2; ld r20,112(r4) +err2; ld r21,120(r4) + addir4,r4,128 +err2; std r0,0(r3) +err
Re: [v3 5/7] powerpc/memcpy_mcsafe: return remaining bytes
Le 05/07/2019 à 23:26, Santosh Sivaraj a écrit : memcpy_mcsafe currently return -EFAULT on a machine check exception, change it to return the remaining bytes that needs to be copied, so that machine check safe copy_to_user can maintain the same behavior as copy_to_user. AFAIU, this behaviour is the expected behaviour for memcpy_mcsafe(). Why implement a different behaviour in patch 3 and then change it here. Can't memcpy_mcsafe() return remaining bytes as expected from patch 3 ? Christophe Signed-off-by: Santosh Sivaraj --- arch/powerpc/lib/memcpy_mcsafe_64.S | 142 1 file changed, 83 insertions(+), 59 deletions(-) diff --git a/arch/powerpc/lib/memcpy_mcsafe_64.S b/arch/powerpc/lib/memcpy_mcsafe_64.S index 50f865db0338..4d8a3d315992 100644 --- a/arch/powerpc/lib/memcpy_mcsafe_64.S +++ b/arch/powerpc/lib/memcpy_mcsafe_64.S @@ -30,11 +30,25 @@ ld r14,STK_REG(R14)(r1) addir1,r1,STACKFRAMESIZE .Ldo_err1: - li r3,-EFAULT + /* Do a byte by byte copy to get the exact remaining size */ + mtctr r7 +100: EX_TABLE(100b, .Ldone) +46: +err1; lbz r0,0(r4) + addir4,r4,1 +err1; stb r0,0(r3) + addir3,r3,1 + bdnz46b + li r3,0 + blr + +.Ldone: + mfctr r3 blr _GLOBAL(memcpy_mcsafe) + mr r7,r5 cmpldi r5,16 blt .Lshort_copy @@ -49,18 +63,21 @@ err1; lbz r0,0(r4) addir4,r4,1 err1; stb r0,0(r3) addir3,r3,1 + subir7,r7,1 1: bf cr7*4+2,2f err1; lhz r0,0(r4) addir4,r4,2 err1; sth r0,0(r3) addir3,r3,2 + subir7,r7,2 2: bf cr7*4+1,3f err1; lwz r0,0(r4) addir4,r4,4 err1; stw r0,0(r3) addir3,r3,4 + subir7,r7,4 3: sub r5,r5,r6 cmpldi r5,128 @@ -87,43 +104,69 @@ err1; stw r0,0(r3) 4: err2; ld r0,0(r4) err2; ld r6,8(r4) -err2; ld r7,16(r4) -err2; ld r8,24(r4) -err2; ld r9,32(r4) -err2; ld r10,40(r4) -err2; ld r11,48(r4) -err2; ld r12,56(r4) -err2; ld r14,64(r4) -err2; ld r15,72(r4) -err2; ld r16,80(r4) -err2; ld r17,88(r4) -err2; ld r18,96(r4) -err2; ld r19,104(r4) -err2; ld r20,112(r4) -err2; ld r21,120(r4) +err2; ld r8,16(r4) +err2; ld r9,24(r4) +err2; ld r10,32(r4) +err2; ld r11,40(r4) +err2; ld r12,48(r4) +err2; ld r14,56(r4) +err2; ld r15,64(r4) +err2; ld r16,72(r4) +err2; ld r17,80(r4) +err2; ld r18,88(r4) +err2; ld r19,96(r4) +err2; ld r20,104(r4) +err2; ld r21,112(r4) +err2; ld r22,120(r4) addir4,r4,128 err2; std r0,0(r3) err2; std r6,8(r3) -err2; std r7,16(r3) -err2; std r8,24(r3) -err2; std r9,32(r3) -err2; std r10,40(r3) -err2; std r11,48(r3) -err2; std r12,56(r3) -err2; std r14,64(r3) -err2; std r15,72(r3) -err2; std r16,80(r3) -err2; std r17,88(r3) -err2; std r18,96(r3) -err2; std r19,104(r3) -err2; std r20,112(r3) -err2; std r21,120(r3) +err2; std r8,16(r3) +err2; std r9,24(r3) +err2; std r10,32(r3) +err2; std r11,40(r3) +err2; std r12,48(r3) +err2; std r14,56(r3) +err2; std r15,64(r3) +err2; std r16,72(r3) +err2; std r17,80(r3) +err2; std r18,88(r3) +err2; std r19,96(r3) +err2; std r20,104(r3) +err2; std r21,112(r3) +err2; std r22,120(r3) addir3,r3,128 + subir7,r7,128 bdnz4b clrldi r5,r5,(64-7) - ld r14,STK_REG(R14)(r1) + /* Up to 127B to go */ +5: srdir6,r5,4 + mtocrf 0x01,r6 + +6: bf cr7*4+1,7f +err2; ld r0,0(r4) +err2; ld r6,8(r4) +err2; ld r8,16(r4) +err2; ld r9,24(r4) +err2; ld r10,32(r4) +err2; ld r11,40(r4) +err2; ld r12,48(r4) +err2; ld r14,56(r4) + addir4,r4,64 +err2; std r0,0(r3) +err2; std r6,8(r3) +err2; std r8,16(r3) +err2; std r9,24(r3) +err2; std r10,32(r3) +err2; std r11,40(r3) +err2; std r12,48(r3) +err2; std r14,56(r3) + addir3,r3,64 + subir7,r7,64 + +7: ld r14,STK_REG(R14)(r1) ld r15,STK_REG(R15)(r1) ld r16,STK_REG(R16)(r1) ld r17,STK_REG(R17)(r1) @@ -134,42 +177,19 @@ err2; std r21,120(r3) ld r22,STK_REG(R22)(r1) addir1,r1,STACKFRAMESIZE - /* Up to 127B to go */ -5: srdir6,r5,4 - mtocrf 0x01,r6 - -6: bf cr7*4+1,7f -err1; ld r0,0(r4) -err1; ld r6,8(r4) -err1; ld r7,16(r4) -err1; ld r8,24(r4) -err1; ld r9,32(r4) -err1; ld r10,40(r4) -err1; ld r11,48(r4) -err1; ld r12,56(r4) - addir4,r4,64 -err1; std r0,0(r3) -err1; std r6,8
Re: [v3 6/7] powerpc: add machine check safe copy_to_user
Le 05/07/2019 à 23:26, Santosh Sivaraj a écrit : Use memcpy_mcsafe() implementation to define copy_to_user_mcsafe() Signed-off-by: Santosh Sivaraj --- arch/powerpc/Kconfig | 1 + arch/powerpc/include/asm/uaccess.h | 12 2 files changed, 13 insertions(+) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 8c1c636308c8..a173b392c272 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -134,6 +134,7 @@ config PPC select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION) select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAS_UACCESS_FLUSHCACHE if PPC64 + select ARCH_HAS_UACCESS_MCSAFE if PPC64 select ARCH_HAS_UBSAN_SANITIZE_ALL select ARCH_HAS_ZONE_DEVICE if PPC_BOOK3S_64 select ARCH_HAVE_NMI_SAFE_CMPXCHG diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h index 76f34346b642..f8fcaab4c5bc 100644 --- a/arch/powerpc/include/asm/uaccess.h +++ b/arch/powerpc/include/asm/uaccess.h @@ -386,6 +386,18 @@ static inline unsigned long raw_copy_to_user(void __user *to, return ret; } +static __always_inline unsigned long __must_check +copy_to_user_mcsafe(void __user *to, const void *from, unsigned long n) +{ + if (likely(check_copy_size(from, n, true))) { + allow_write_to_user(to, n); + n = memcpy_mcsafe(to, from, n); memcpy_mcsafe() doesn't expect a __user pointer. I think at least a cast is needed. And should we perform an access_ok() verification too ? Christophe + prevent_write_to_user(to, n); + } + + return n; +} + extern unsigned long __clear_user(void __user *addr, unsigned long size); static inline unsigned long clear_user(void __user *addr, unsigned long size)