* Re: [PATCH v3] powerpc/powernv: add NULL check after kzalloc in opal_add_one_export
From: Qiujun Huang @ 2020-04-06 10:40 UTC (permalink / raw)
To: Markus Elfring
Cc: LKML, Oliver O'Halloran, Paul Mackerras, Thomas Gleixner,
linuxppc-dev
In-Reply-To: <d8334f5d-403f-941f-abcc-2714d297082b@web.de>
On Mon, Apr 6, 2020 at 6:02 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> >>>> Here needs a NULL check.
> >> quite obvious?
>
> I suggest to consider another fine-tuning for the wording also around
> such “obvious” programming items.
>
>
> >>> I find this change description questionable
> >>> (despite of a reasonable patch subject).
>
> I got further development concerns.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=a10c9c710f9ecea87b9f4bbb837467893b4bef01#n129
>
> * Were changes mixed for different issues according to the diff code?
>
> * I find it safer here to split specific changes into separate update steps
> for a small patch series.
>
> * Will the addition of the desired null pointer check qualify for
> the specification of the tag “Fixes”?
>
>
> >>> Will a patch change log be helpful here?
> >> I realized I should write some change log, and the change log was meaningless.
>
> Will any more adjustments happen for the discussed update suggestion
> after the third patch version?
>
>
> > The changelog is fine IMO. The point of a changelog is to tell a
> > reader doing git archeology why a change happened and this is
> > sufficent for that.
>
> We might stumble on a different understanding for the affected “change logs”.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=a10c9c710f9ecea87b9f4bbb837467893b4bef01#n751
>
> Would you like to follow the patch evolution a bit easier?
>
> Regards,
> Markus
Thanks for the reply.
I should study the documentation first.
BTW, happy new week
^ permalink raw reply
* Re: [PATCH v4] powerpc/powernv: add NULL check after kzalloc in opal_add_one_export
From: Qiujun Huang @ 2020-04-06 10:43 UTC (permalink / raw)
To: Michael Ellerman
Cc: LKML, Oliver O'Halloran, Paul Mackerras, Thomas Gleixner,
linuxppc-dev, Markus Elfring
In-Reply-To: <87eet0sy79.fsf@mpe.ellerman.id.au>
On Mon, Apr 6, 2020 at 6:30 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Qiujun Huang <hqjagain@gmail.com> writes:
> > Here needs a NULL check as kzalloc may fail returning NULL.
> >
> > Issue was found by coccinelle.
> > Generated by: scripts/coccinelle/null/kmerr.cocci
> >
> > Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
> > Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
> >
> > ---
>
> Thanks for putting up with all the review comments :)
>
> But I think this should actually be two patches now.
>
> The first patch should change the goto after
> of_property_read_u64_array() into a return and drop the redundant
> assignments.
>
> Then the second patch can add the NULL check for attr.
Get that, I'll separate them.
>
> cheers
>
> > v3->v4:
> > Added the information about coccinelle script.
> > Added change log.
> > Added Oliver's Reviewed-by.
> > v2->v3:
> > Removed redundant assignment to 'attr' and 'name'.
> > v1->v2:
> > Just return -ENOMEM if attr is NULL.
> > ---
> > arch/powerpc/platforms/powernv/opal.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> > index 2b3dfd0b6cdd..908d749bcef5 100644
> > --- a/arch/powerpc/platforms/powernv/opal.c
> > +++ b/arch/powerpc/platforms/powernv/opal.c
> > @@ -801,16 +801,19 @@ static ssize_t export_attr_read(struct file *fp, struct kobject *kobj,
> > static int opal_add_one_export(struct kobject *parent, const char *export_name,
> > struct device_node *np, const char *prop_name)
> > {
> > - struct bin_attribute *attr = NULL;
> > - const char *name = NULL;
> > + struct bin_attribute *attr;
> > + const char *name;
> > u64 vals[2];
> > int rc;
> >
> > rc = of_property_read_u64_array(np, prop_name, &vals[0], 2);
> > if (rc)
> > - goto out;
> > + return rc;
> >
> > attr = kzalloc(sizeof(*attr), GFP_KERNEL);
> > + if (!attr)
> > + return -ENOMEM;
> > +
> > name = kstrdup(export_name, GFP_KERNEL);
> > if (!name) {
> > rc = -ENOMEM;
> > --
> > 2.17.1
^ permalink raw reply
* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.7-1 tag
From: Michael Ellerman @ 2020-04-06 10:57 UTC (permalink / raw)
To: Linus Torvalds
Cc: tyreld, shilpa.bhat, gustavold, aik, Nick Desaulniers, psampat,
bala24, Grant Likely, Oliver O'Halloran, afzal.mohd.ma,
srikar, Stephen Rothwell, Joe Lawrence, maskray, Ilie Halip,
Aneesh Kumar K.V, YueHaibing, Mike Rapoport, chenzhou10, ganeshgr,
dougmill, kjain, leonardo, Naveen N. Rao, agust, laurentiu.tudor,
nathanl, Arnd Bergmann, alistair, Nick Piggin, oss,
Olof Johansson, maddy, Christophe JAILLET, clg, courbet, vaibhav,
Bjorn Helgaas, Nathan Chancellor, Daniel Axtens, farosas,
Greg Kroah-Hartman, lpechacek, Linux Kernel Mailing List,
sourabhjain, Joe Perches, po-hsu.lin, linuxppc-dev
In-Reply-To: <CAHk-=wgkkmNV5tMzQDmPAQuNJBuMcry--Jb+h8H1o4RA3kF7QQ@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Sun, Apr 5, 2020 at 5:53 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> There is one conflict in fs/sysfs/group.c, between our:
>>
>> 9255782f7061 ("sysfs: Wrap __compat_only_sysfs_link_entry_to_kobj function to change the symlink name")
> [...]
>
> The conflict was trivial.
>
> But I want to kvetch a bit about that commit. It's doing some odd stuff.
>
> In particular, it's wrapping things "the wrong way". Our naming rules
> are that the double underscore versions are the internal helper
> functions that you generally shouldn't use unless you have some extra
> reason for it, and then the non-underscore versions are the preferred
> and simpler user interface to those internal implementations.
>
> IOW, the _wrapper_ doesn't have double underscores, it's the _wrappee_
> that has the underscores.
>
> That commit does the exact reverse of that usual pattern, which is
> very confusing.
>
> Now, I see _why_ you do that - normally the non-underscore version is
> the "real" interface and the one we've always exported, and then the
> double underscore is the special internal thing that maybe exposes
> some internal detail (or maybe only does one special case of it and
> leaves out locking or whatever).
>
> In this case, for hysterical raisins, we only _had_ that
> double-underscore version, and you basically added the new case and
> did it without the underscores.
>
> So I see why it happened the way it did, but I do think the end result
> makes no sense and is odd and surprising.
Yeah, that's fair.
I was a bit unsure about taking a fs/sysfs patch to begin with, so I
thought leaving the existing function unchanged was the least risky in
terms of causing any other breakage.
But in hindsight that was the wrong choice, the end result is not
actually what we want.
So we should have done the right patch and then either asked Greg to
take it or put it in a topic branch of my own.
> The thing is, we have exactly *one* user of that double-underscore
> version: tpm-chip.c (ok, there are two calls in that file, but it's a
> single user).
>
> So I think it should just have removed the __ version entirely. Make
> tpm-chip just use the new semantics, and pass in the extra NULL
> argument.
>
> I guess I'll just do that as a cleanup patch on top, but it feels a
> bit odd to have to do that cleanup when the original patch could have
> just done the obvious thing.
Thanks.
cheers
^ permalink raw reply
* Re: [PATCH v5 18/21] powerpc64: Add prefixed instructions to instruction data type
From: Jordan Niethe @ 2020-04-06 10:42 UTC (permalink / raw)
To: Alistair Popple; +Cc: npiggin, bala24, linuxppc-dev, dja
In-Reply-To: <7182352.hY56U9iWWN@townsend>
[-- Attachment #1: Type: text/plain, Size: 18341 bytes --]
On Mon, 6 Apr 2020, 7:52 pm Alistair Popple, <alistair@popple.id.au> wrote:
> > diff --git a/arch/powerpc/include/asm/inst.h
> > b/arch/powerpc/include/asm/inst.h index 70b37a35a91a..7e23e7146c66 100644
> > --- a/arch/powerpc/include/asm/inst.h
> > +++ b/arch/powerpc/include/asm/inst.h
> > @@ -8,23 +8,67 @@
> >
> > struct ppc_inst {
> > u32 val;
> > +#ifdef __powerpc64__
> > + u32 suffix;
> > +#endif /* __powerpc64__ */
> > } __packed;
> >
> > -#define ppc_inst(x) ((struct ppc_inst){ .val = x })
> > +static inline int ppc_inst_opcode(struct ppc_inst x)
> > +{
> > + return x.val >> 26;
> > +}
> >
> > static inline u32 ppc_inst_val(struct ppc_inst x)
> > {
> > return x.val;
> > }
> >
> > -static inline bool ppc_inst_len(struct ppc_inst x)
> > +#ifdef __powerpc64__
> > +#define ppc_inst(x) ((struct ppc_inst){ .val = (x), .suffix = 0xff })
> > +
> > +#define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix =
> (y)
> > }) +
> > +static inline u32 ppc_inst_suffix(struct ppc_inst x)
> > {
> > - return sizeof(struct ppc_inst);
> > + return x.suffix;
> > }
> >
> > -static inline int ppc_inst_opcode(struct ppc_inst x)
> > +static inline bool ppc_inst_prefixed(struct ppc_inst x) {
> > + return ((ppc_inst_val(x) >> 26) == 1) && ppc_inst_suffix(x) !=
> 0xff;
> > +}
> > +
> > +static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> > {
> > - return x.val >> 26;
> > + return ppc_inst_prefix(swab32(ppc_inst_val(x)),
> > + swab32(ppc_inst_suffix(x)));
> > +}
> > +
> > +static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> > +{
> > + u32 val, suffix = 0xff;
> > + val = *(u32 *)ptr;
> > + if ((val >> 26) == 1)
> > + suffix = *((u32 *)ptr + 1);
> > + return ppc_inst_prefix(val, suffix);
> > +}
> > +
> > +static inline void ppc_inst_write(struct ppc_inst *ptr, struct ppc_inst
> x)
> > +{
> > + if (ppc_inst_prefixed(x)) {
> > + *(u32 *)ptr = x.val;
> > + *((u32 *)ptr + 1) = x.suffix;
> > + } else {
> > + *(u32 *)ptr = x.val;
> > + }
> > +}
> > +
> > +#else
> > +
> > +#define ppc_inst(x) ((struct ppc_inst){ .val = x })
> > +
> > +static inline bool ppc_inst_prefixed(ppc_inst x)
> > +{
> > + return 0;
> > }
> >
> > static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> > @@ -32,14 +76,31 @@ static inline struct ppc_inst ppc_inst_swab(struct
> > ppc_inst x) return ppc_inst(swab32(ppc_inst_val(x)));
> > }
> >
> > +static inline u32 ppc_inst_val(struct ppc_inst x)
> > +{
> > + return x.val;
> > +}
> > +
> > static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> > {
> > return *ptr;
> > }
> >
> > +static inline void ppc_inst_write(struct ppc_inst *ptr, struct ppc_inst
> x)
> > +{
> > + *ptr = x;
> > +}
> > +
> > +#endif /* __powerpc64__ */
> > +
> > static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
> > {
> > return !memcmp(&x, &y, sizeof(struct ppc_inst));
> > }
>
> Apologies for not picking this up earlier, I was hoping to get to the
> bottom
> of the issue I was seeing before you sent out v5. However the above
> definition
> of instruction equality does not seem correct because it does not consider
> the
> case when an instruction is not prefixed - a non-prefixed instruction
> should be
> considered equal if the first 32-bit opcode/value is the same. Something
> like:
>
> if (ppc_inst_prefixed(x) != ppc_inst_prefixed(y))
> return false;
> else if (ppc_inst_prefixed(x))
> return !memcmp(&x, &y, sizeof(struct ppc_inst));
> else
> return x.val == y.val;
>
> This was causing failures in ftrace_modify_code() as it would falsely
> detect
> two non-prefixed instructions as being not equal due to differences in the
> suffix.
>
Hm I was intending that non prefixed instructions would always have the
suffix set to the same value. If that's not happening, something must be
wrong with where the instructions are created.
>
> - Alistair
>
> > +static inline int ppc_inst_len(struct ppc_inst x)
> > +{
> > + return (ppc_inst_prefixed(x)) ? 8 : 4;
> > +}
> > +
> > #endif /* _ASM_INST_H */
> > diff --git a/arch/powerpc/include/asm/kprobes.h
> > b/arch/powerpc/include/asm/kprobes.h index 66b3f2983b22..4fc0e15e23a5
> > 100644
> > --- a/arch/powerpc/include/asm/kprobes.h
> > +++ b/arch/powerpc/include/asm/kprobes.h
> > @@ -43,7 +43,7 @@ extern kprobe_opcode_t optprobe_template_ret[];
> > extern kprobe_opcode_t optprobe_template_end[];
> >
> > /* Fixed instruction size for powerpc */
> > -#define MAX_INSN_SIZE 1
> > +#define MAX_INSN_SIZE 2
> > #define MAX_OPTIMIZED_LENGTH sizeof(kprobe_opcode_t) /* 4 bytes */
> > #define MAX_OPTINSN_SIZE (optprobe_template_end -
> optprobe_template_entry)
> > #define RELATIVEJUMP_SIZE sizeof(kprobe_opcode_t) /* 4 bytes */
> > diff --git a/arch/powerpc/include/asm/uaccess.h
> > b/arch/powerpc/include/asm/uaccess.h index c0a35e4586a5..5a3f486ddf02
> > 100644
> > --- a/arch/powerpc/include/asm/uaccess.h
> > +++ b/arch/powerpc/include/asm/uaccess.h
> > @@ -105,11 +105,34 @@ static inline int __access_ok(unsigned long addr,
> > unsigned long size, #define __put_user_inatomic(x, ptr) \
> > __put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
> >
> > -#define __get_user_instr(x, ptr) \
> > - __get_user_nocheck((x).val, (u32 *)(ptr), sizeof(u32), true)
> > +#define __get_user_instr(x, ptr) \
> > +({ \
> > + long __gui_ret = 0; \
> > + unsigned int prefix, suffix; \
> > + __gui_ret = __get_user(prefix, (unsigned int __user *)ptr);
> \
> > + if (!__gui_ret && (prefix >> 26) == 1) { \
> > + __gui_ret = __get_user(suffix, (unsigned int __user *)ptr
> + 1); \
> > + (x) = ppc_inst_prefix(prefix, suffix); \
> > + } else { \
> > + (x) = ppc_inst(prefix); \
> > + } \
> > + __gui_ret; \
> > +})
> > +
> > +#define __get_user_instr_inatomic(x, ptr) \
> > +({ \
> > + long __gui_ret = 0; \
> > + unsigned int prefix, suffix; \
> > + __gui_ret = __get_user_inatomic(prefix, (unsigned int __user
> *)ptr); \
> > + if (!__gui_ret && (prefix >> 26) == 1) { \
> > + __gui_ret = __get_user_inatomic(suffix, (unsigned int
> __user *)ptr +
> > 1); \ + (x) = ppc_inst_prefix(prefix, suffix); \
> > + } else { \
> > + (x) = ppc_inst(prefix); \
> > + } \
> > + __gui_ret; \
> > +})
> >
> > -#define __get_user_instr_inatomic(x, ptr) \
> > - __get_user_nosleep((x).val, (u32 *)(ptr), sizeof(u32))
> > extern long __put_user_bad(void);
> >
> > /*
> > diff --git a/arch/powerpc/include/asm/uprobes.h
> > b/arch/powerpc/include/asm/uprobes.h index 7e3b329ba2d3..5bf65f5d44a9
> > 100644
> > --- a/arch/powerpc/include/asm/uprobes.h
> > +++ b/arch/powerpc/include/asm/uprobes.h
> > @@ -15,7 +15,7 @@
> >
> > typedef ppc_opcode_t uprobe_opcode_t;
> >
> > -#define MAX_UINSN_BYTES 4
> > +#define MAX_UINSN_BYTES 8
> > #define UPROBE_XOL_SLOT_BYTES (MAX_UINSN_BYTES)
> >
> > /* The following alias is needed for reference from arch-agnostic code
> */
> > diff --git a/arch/powerpc/kernel/optprobes.c
> > b/arch/powerpc/kernel/optprobes.c index 684640b8fa2e..689daf430161 100644
> > --- a/arch/powerpc/kernel/optprobes.c
> > +++ b/arch/powerpc/kernel/optprobes.c
> > @@ -159,38 +159,38 @@ void patch_imm32_load_insns(unsigned int val,
> > kprobe_opcode_t *addr)
> >
> > /*
> > * Generate instructions to load provided immediate 64-bit value
> > - * to register 'r3' and patch these instructions at 'addr'.
> > + * to register 'reg' and patch these instructions at 'addr'.
> > */
> > -void patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
> > +void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t
> > *addr) {
> > - /* lis r3,(op)@highest */
> > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ADDIS
> |
> > ___PPC_RT(3) | + /* lis reg,(op)@highest */
> > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ADDIS
> |
> > ___PPC_RT(reg) | ((val >> 48) & 0xffff)));
> > addr++;
> >
> > - /* ori r3,r3,(op)@higher */
> > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > ___PPC_RA(3) | - ___PPC_RS(3) | ((val >> 32) &
> 0xffff)));
> > + /* ori reg,reg,(op)@higher */
> > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > ___PPC_RA(reg) | + ___PPC_RS(reg) | ((val >> 32) &
> 0xffff)));
> > addr++;
> >
> > - /* rldicr r3,r3,32,31 */
> > - patch_instruction((struct ppc_inst *)addr,
> ppc_inst(PPC_INST_RLDICR |
> > ___PPC_RA(3) | - ___PPC_RS(3) | __PPC_SH64(32) |
> __PPC_ME64(31)));
> > + /* rldicr reg,reg,32,31 */
> > + patch_instruction((struct ppc_inst *)addr,
> ppc_inst(PPC_INST_RLDICR |
> > ___PPC_RA(reg) | + ___PPC_RS(reg) | __PPC_SH64(32)
> |
> __PPC_ME64(31)));
> > addr++;
> >
> > - /* oris r3,r3,(op)@h */
> > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORIS |
> > ___PPC_RA(3) | - ___PPC_RS(3) | ((val >> 16) &
> 0xffff)));
> > + /* oris reg,reg,(op)@h */
> > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORIS |
> > ___PPC_RA(reg) | + ___PPC_RS(reg) | ((val >> 16) &
> 0xffff)));
> > addr++;
> >
> > - /* ori r3,r3,(op)@l */
> > - patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > ___PPC_RA(3) | - ___PPC_RS(3) | (val & 0xffff)));
> > + /* ori reg,reg,(op)@l */
> > + patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > ___PPC_RA(reg) | + ___PPC_RS(reg) | (val &
> 0xffff)));
> > }
> >
> > int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct
> > kprobe *p) {
> > - struct ppc_inst branch_op_callback, branch_emulate_step;
> > + struct ppc_inst branch_op_callback, branch_emulate_step, temp;
> > kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
> > long b_offset;
> > unsigned long nip, size;
> > @@ -240,7 +240,7 @@ int arch_prepare_optimized_kprobe(struct
> > optimized_kprobe *op, struct kprobe *p) * Fixup the template with
> > instructions to:
> > * 1. load the address of the actual probepoint
> > */
> > - patch_imm64_load_insns((unsigned long)op, buff + TMPL_OP_IDX);
> > + patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
> >
> > /*
> > * 2. branch to optimized_callback() and emulate_step()
> > @@ -271,7 +271,11 @@ int arch_prepare_optimized_kprobe(struct
> > optimized_kprobe *op, struct kprobe *p) /*
> > * 3. load instruction to be emulated into relevant register, and
> > */
> > - patch_imm32_load_insns(*p->ainsn.insn, buff + TMPL_INSN_IDX);
> > + temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
> > + patch_imm64_load_insns(ppc_inst_val(temp) |
> > + ((u64)ppc_inst_suffix(temp) << 32),
> > + 4,
> > + buff + TMPL_INSN_IDX);
> >
> > /*
> > * 4. branch back from trampoline
> > diff --git a/arch/powerpc/kernel/optprobes_head.S
> > b/arch/powerpc/kernel/optprobes_head.S index cf383520843f..ff8ba4d3824d
> > 100644
> > --- a/arch/powerpc/kernel/optprobes_head.S
> > +++ b/arch/powerpc/kernel/optprobes_head.S
> > @@ -94,6 +94,9 @@ optprobe_template_insn:
> > /* 2, Pass instruction to be emulated in r4 */
> > nop
> > nop
> > + nop
> > + nop
> > + nop
> >
> > .global optprobe_template_call_emulate
> > optprobe_template_call_emulate:
> > diff --git a/arch/powerpc/kernel/trace/ftrace.c
> > b/arch/powerpc/kernel/trace/ftrace.c index e78742613b36..16041a5c86d5
> > 100644
> > --- a/arch/powerpc/kernel/trace/ftrace.c
> > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > @@ -41,11 +41,35 @@
> > #define NUM_FTRACE_TRAMPS 8
> > static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
> >
> > +#ifdef __powerpc64__
> > static long
> > probe_kernel_read_inst(struct ppc_inst *inst, const void *src)
> > {
> > - return probe_kernel_read((void *)inst, src, MCOUNT_INSN_SIZE);
> > + u32 val, suffix = 0;
> > + long err;
> > +
> > + err = probe_kernel_read((void *)&val,
> > + src, sizeof(val));
> > + if (err)
> > + return err;
> > +
> > + if ((val >> 26) == 1)
> > + err = probe_kernel_read((void *)&suffix,
> > + src + 4, MCOUNT_INSN_SIZE);
> > + if (err)
> > + return err;
> > +
> > + *inst = ppc_inst_prefix(val, suffix);
> > +
> > + return 0;
> > }
> > +#else
> > +static long
> > +probe_kernel_read_inst(struct ppc_inst *inst, const void *src)
> > +{
> > + return probe_kernel_read((void *)inst, src, MCOUNT_INSN_SIZE)
> > +}
> > +#endif
> >
> > static struct ppc_inst
> > ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
> > diff --git a/arch/powerpc/lib/code-patching.c
> > b/arch/powerpc/lib/code-patching.c index c329ad657302..b4007e03d8fa
> 100644
> > --- a/arch/powerpc/lib/code-patching.c
> > +++ b/arch/powerpc/lib/code-patching.c
> > @@ -24,12 +24,19 @@ static int __patch_instruction(struct ppc_inst
> > *exec_addr, struct ppc_inst instr {
> > int err = 0;
> >
> > - __put_user_asm(ppc_inst_val(instr), patch_addr, err, "stw");
> > - if (err)
> > - return err;
> > -
> > - asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r"
> (patch_addr),
> > - "r"
> (exec_addr));
> > + if (!ppc_inst_prefixed(instr)) {
> > + __put_user_asm(ppc_inst_val(instr), patch_addr, err,
> "stw");
> > + if (err)
> > + return err;
> > + asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r"
> (patch_addr),
> > + "r"
> (exec_addr));
> > + } else {
> > + __put_user_asm((u64)ppc_inst_suffix(instr) << 32 |
> ppc_inst_val(instr),
> > patch_addr, err, "std"); + if (err)
> > + return err;
> > + asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r"
> (patch_addr),
> > + "r"
> (exec_addr));
> > + }
> >
> > return 0;
> > }
> > diff --git a/arch/powerpc/lib/feature-fixups.c
> > b/arch/powerpc/lib/feature-fixups.c index f00dd13b1c3c..5519cec83cc8
> 100644
> > --- a/arch/powerpc/lib/feature-fixups.c
> > +++ b/arch/powerpc/lib/feature-fixups.c
> > @@ -84,12 +84,13 @@ static int patch_feature_section(unsigned long value,
> > struct fixup_entry *fcur) src = alt_start;
> > dest = start;
> >
> > - for (; src < alt_end; src++, dest++) {
> > + for (; src < alt_end; src = (void *)src +
> > ppc_inst_len(ppc_inst_read(src)), + (dest = (void *)dest +
> > ppc_inst_len(ppc_inst_read(dest)))) { if (patch_alt_instruction(src,
> dest,
> > alt_start, alt_end))
> > return 1;
> > }
> >
> > - for (; dest < end; dest++)
> > + for (; dest < end; dest = (void *)dest +
> > ppc_inst_len(ppc_inst(PPC_INST_NOP))) raw_patch_instruction(dest,
> > ppc_inst(PPC_INST_NOP));
> >
> > return 0;
> > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > index 52ddd3122dc8..8b285bf11218 100644
> > --- a/arch/powerpc/lib/sstep.c
> > +++ b/arch/powerpc/lib/sstep.c
> > @@ -1169,10 +1169,12 @@ int analyse_instr(struct instruction_op *op,
> const
> > struct pt_regs *regs, unsigned long int imm;
> > unsigned long int val, val2;
> > unsigned int mb, me, sh;
> > - unsigned int word;
> > + unsigned int word, suffix;
> > long ival;
> >
> > word = ppc_inst_val(instr);
> > + suffix = ppc_inst_suffix(instr);
> > +
> > op->type = COMPUTE;
> >
> > opcode = word >> 26;
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index 6f3bcdcfc9c7..b704aebb099a 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -761,8 +761,8 @@ static int xmon_bpt(struct pt_regs *regs)
> >
> > /* Are we at the trap at bp->instr[1] for some bp? */
> > bp = in_breakpoint_table(regs->nip, &offset);
> > - if (bp != NULL && offset == 4) {
> > - regs->nip = bp->address + 4;
> > + if (bp != NULL && (offset == 4 || offset == 8)) {
> > + regs->nip = bp->address + offset;
> > atomic_dec(&bp->ref_count);
> > return 1;
> > }
> > @@ -863,7 +863,7 @@ static struct bpt *in_breakpoint_table(unsigned long
> > nip, unsigned long *offp) if (off >= sizeof(bpt_table))
> > return NULL;
> > *offp = off % BPT_SIZE;
> > - if (*offp != 0 && *offp != 4)
> > + if (*offp != 0 && *offp != 4 && *offp != 8)
> > return NULL;
> > return bpts + (off / BPT_SIZE);
> > }
> > diff --git a/arch/powerpc/xmon/xmon_bpts.S
> b/arch/powerpc/xmon/xmon_bpts.S
> > index ebb2dbc70ca8..09058eb6abbd 100644
> > --- a/arch/powerpc/xmon/xmon_bpts.S
> > +++ b/arch/powerpc/xmon/xmon_bpts.S
> > @@ -3,6 +3,8 @@
> > #include <asm/asm-compat.h>
> > #include "xmon_bpts.h"
> >
> > +/* Prefixed instructions can not cross 64 byte boundaries */
> > +.align 6
> > .global bpt_table
> > bpt_table:
> > - .space NBPTS * 8
> > + .space NBPTS * 16
>
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 23221 bytes --]
^ permalink raw reply
* Re: [PATCH v5 18/21] powerpc64: Add prefixed instructions to instruction data type
From: kbuild test robot @ 2020-04-06 11:04 UTC (permalink / raw)
To: Jordan Niethe
Cc: kbuild-all, Jordan Niethe, npiggin, bala24, alistair,
linuxppc-dev, dja
In-Reply-To: <20200406080936.7180-19-jniethe5@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 14164 bytes --]
Hi Jordan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v5.6]
[cannot apply to powerpc/next kvm-ppc/kvm-ppc-next scottwood/next next-20200406]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Jordan-Niethe/Initial-Prefixed-Instruction-support/20200406-165215
base: 7111951b8d4973bda27ff663f2cf18b663d15b48
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from arch/powerpc/include/asm/sstep.h:5,
from arch/powerpc/kernel/align.c:26:
>> arch/powerpc/include/asm/inst.h:69:38: error: unknown type name 'ppc_inst'
69 | static inline bool ppc_inst_prefixed(ppc_inst x)
| ^~~~~~~~
>> arch/powerpc/include/asm/inst.h:79:19: error: redefinition of 'ppc_inst_val'
79 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~~~~~~~~~
arch/powerpc/include/asm/inst.h:21:19: note: previous definition of 'ppc_inst_val' was here
21 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~~~~~~~~~
arch/powerpc/include/asm/inst.h: In function 'ppc_inst_len':
>> arch/powerpc/include/asm/inst.h:103:10: error: implicit declaration of function 'ppc_inst_prefixed'; did you mean 'ppc_inst_write'? [-Werror=implicit-function-declaration]
103 | return (ppc_inst_prefixed(x)) ? 8 : 4;
| ^~~~~~~~~~~~~~~~~
| ppc_inst_write
In file included from include/linux/kernel.h:11,
from arch/powerpc/kernel/align.c:16:
arch/powerpc/kernel/align.c: In function 'fix_alignment':
>> arch/powerpc/include/asm/uaccess.h:115:9: error: implicit declaration of function 'ppc_inst_prefix'; did you mean 'ppc_inst_read'? [-Werror=implicit-function-declaration]
115 | (x) = ppc_inst_prefix(prefix, suffix); \
| ^~~~~~~~~~~~~~~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
>> arch/powerpc/kernel/align.c:307:15: note: in expansion of macro '__get_user_instr'
307 | if (unlikely(__get_user_instr(instr, (void __user *)regs->nip)))
| ^~~~~~~~~~~~~~~~
>> arch/powerpc/include/asm/uaccess.h:115:9: error: incompatible types when assigning to type 'struct ppc_inst' from type 'int'
115 | (x) = ppc_inst_prefix(prefix, suffix); \
| ^~~~~~~~~~~~~~~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
>> arch/powerpc/kernel/align.c:307:15: note: in expansion of macro '__get_user_instr'
307 | if (unlikely(__get_user_instr(instr, (void __user *)regs->nip)))
| ^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
--
In file included from arch/powerpc/include/asm/code-patching.h:14,
from arch/powerpc/kernel/process.c:60:
>> arch/powerpc/include/asm/inst.h:69:38: error: unknown type name 'ppc_inst'
69 | static inline bool ppc_inst_prefixed(ppc_inst x)
| ^~~~~~~~
>> arch/powerpc/include/asm/inst.h:79:19: error: redefinition of 'ppc_inst_val'
79 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~~~~~~~~~
arch/powerpc/include/asm/inst.h:21:19: note: previous definition of 'ppc_inst_val' was here
21 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~~~~~~~~~
arch/powerpc/include/asm/inst.h: In function 'ppc_inst_len':
>> arch/powerpc/include/asm/inst.h:103:10: error: implicit declaration of function 'ppc_inst_prefixed'; did you mean 'ppc_inst_write'? [-Werror=implicit-function-declaration]
103 | return (ppc_inst_prefixed(x)) ? 8 : 4;
| ^~~~~~~~~~~~~~~~~
| ppc_inst_write
cc1: all warnings being treated as errors
--
In file included from arch/powerpc/include/asm/code-patching.h:14,
from arch/powerpc/kernel/setup_32.c:42:
>> arch/powerpc/include/asm/inst.h:69:38: error: unknown type name 'ppc_inst'
69 | static inline bool ppc_inst_prefixed(ppc_inst x)
| ^~~~~~~~
>> arch/powerpc/include/asm/inst.h:79:19: error: redefinition of 'ppc_inst_val'
79 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~~~~~~~~~
arch/powerpc/include/asm/inst.h:21:19: note: previous definition of 'ppc_inst_val' was here
21 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~~~~~~~~~
arch/powerpc/include/asm/inst.h: In function 'ppc_inst_len':
>> arch/powerpc/include/asm/inst.h:103:10: error: implicit declaration of function 'ppc_inst_prefixed'; did you mean 'ppc_inst_write'? [-Werror=implicit-function-declaration]
103 | return (ppc_inst_prefixed(x)) ? 8 : 4;
| ^~~~~~~~~~~~~~~~~
| ppc_inst_write
In file included from arch/powerpc/include/asm/asm-compat.h:6,
from arch/powerpc/include/asm/bitops.h:42,
from include/linux/bitops.h:29,
from include/linux/kernel.h:12,
from include/linux/list.h:9,
from include/linux/module.h:12,
from arch/powerpc/kernel/setup_32.c:6:
arch/powerpc/kernel/setup_32.c: In function 'machine_init':
arch/powerpc/include/asm/ppc-opcode.h:234:24: error: incompatible type for argument 2 of 'patch_instruction_site'
234 | #define PPC_INST_NOP 0x60000000
| ^~~~~~~~~~
| |
| int
arch/powerpc/kernel/setup_32.c:89:49: note: in expansion of macro 'PPC_INST_NOP'
89 | patch_instruction_site(&patch__memcpy_nocache, PPC_INST_NOP);
| ^~~~~~~~~~~~
In file included from arch/powerpc/kernel/setup_32.c:42:
arch/powerpc/include/asm/code-patching.h:39:69: note: expected 'struct ppc_inst' but argument is of type 'int'
39 | static inline int patch_instruction_site(s32 *site, struct ppc_inst instr)
| ~~~~~~~~~~~~~~~~^~~~~
arch/powerpc/kernel/setup_32.c:91:48: error: passing argument 1 of 'branch_target' from incompatible pointer type [-Werror=incompatible-pointer-types]
91 | create_cond_branch(&insn, addr, branch_target(addr), 0x820000);
| ^~~~
| |
| unsigned int *
In file included from arch/powerpc/kernel/setup_32.c:42:
arch/powerpc/include/asm/code-patching.h:63:52: note: expected 'const struct ppc_inst *' but argument is of type 'unsigned int *'
63 | unsigned long branch_target(const struct ppc_inst *instr);
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~~
arch/powerpc/kernel/setup_32.c:91:28: error: passing argument 2 of 'create_cond_branch' from incompatible pointer type [-Werror=incompatible-pointer-types]
91 | create_cond_branch(&insn, addr, branch_target(addr), 0x820000);
| ^~~~
| |
| unsigned int *
In file included from arch/powerpc/kernel/setup_32.c:42:
arch/powerpc/include/asm/code-patching.h:28:71: note: expected 'const struct ppc_inst *' but argument is of type 'unsigned int *'
28 | int create_cond_branch(struct ppc_inst *instr, const struct ppc_inst *addr,
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
arch/powerpc/kernel/setup_32.c:92:20: error: passing argument 1 of 'patch_instruction' from incompatible pointer type [-Werror=incompatible-pointer-types]
92 | patch_instruction(addr, insn); /* replace b by bne cr0 */
| ^~~~
| |
| unsigned int *
In file included from arch/powerpc/kernel/setup_32.c:42:
arch/powerpc/include/asm/code-patching.h:31:40: note: expected 'struct ppc_inst *' but argument is of type 'unsigned int *'
31 | int patch_instruction(struct ppc_inst *addr, struct ppc_inst instr);
| ~~~~~~~~~~~~~~~~~^~~~
cc1: all warnings being treated as errors
--
In file included from arch/powerpc/include/asm/code-patching.h:14,
from arch/powerpc/include/asm/kprobes.h:24,
from include/linux/kprobes.h:30,
from arch/powerpc/lib/code-patching.c:7:
>> arch/powerpc/include/asm/inst.h:69:38: error: unknown type name 'ppc_inst'
69 | static inline bool ppc_inst_prefixed(ppc_inst x)
| ^~~~~~~~
>> arch/powerpc/include/asm/inst.h:79:19: error: redefinition of 'ppc_inst_val'
79 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~~~~~~~~~
arch/powerpc/include/asm/inst.h:21:19: note: previous definition of 'ppc_inst_val' was here
21 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~~~~~~~~~
arch/powerpc/include/asm/inst.h: In function 'ppc_inst_len':
>> arch/powerpc/include/asm/inst.h:103:10: error: implicit declaration of function 'ppc_inst_prefixed'; did you mean 'ppc_inst_write'? [-Werror=implicit-function-declaration]
103 | return (ppc_inst_prefixed(x)) ? 8 : 4;
| ^~~~~~~~~~~~~~~~~
| ppc_inst_write
In file included from include/linux/uaccess.h:11,
from arch/powerpc/include/asm/sections.h:7,
from include/linux/kallsyms.h:15,
from include/linux/ftrace.h:11,
from include/linux/kprobes.h:29,
from arch/powerpc/lib/code-patching.c:7:
arch/powerpc/lib/code-patching.c: In function '__patch_instruction':
>> arch/powerpc/lib/code-patching.c:34:23: error: implicit declaration of function 'ppc_inst_suffix'; did you mean 'ppc_inst_swab'? [-Werror=implicit-function-declaration]
34 | __put_user_asm((u64)ppc_inst_suffix(instr) << 32 | ppc_inst_val(instr), patch_addr, err, "std");
| ^~~~~~~~~~~~~~~
arch/powerpc/include/asm/uaccess.h:153:10: note: in definition of macro '__put_user_asm'
153 | : "r" (x), "b" (addr), "i" (-EFAULT), "0" (err))
| ^
cc1: all warnings being treated as errors
--
In file included from arch/powerpc/include/asm/code-patching.h:14,
from arch/powerpc/include/asm/kprobes.h:24,
from include/linux/kprobes.h:30,
from arch/powerpc/lib/sstep.c:8:
>> arch/powerpc/include/asm/inst.h:69:38: error: unknown type name 'ppc_inst'
69 | static inline bool ppc_inst_prefixed(ppc_inst x)
| ^~~~~~~~
>> arch/powerpc/include/asm/inst.h:79:19: error: redefinition of 'ppc_inst_val'
79 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~~~~~~~~~
arch/powerpc/include/asm/inst.h:21:19: note: previous definition of 'ppc_inst_val' was here
21 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~~~~~~~~~
arch/powerpc/include/asm/inst.h: In function 'ppc_inst_len':
>> arch/powerpc/include/asm/inst.h:103:10: error: implicit declaration of function 'ppc_inst_prefixed'; did you mean 'ppc_inst_write'? [-Werror=implicit-function-declaration]
103 | return (ppc_inst_prefixed(x)) ? 8 : 4;
| ^~~~~~~~~~~~~~~~~
| ppc_inst_write
arch/powerpc/lib/sstep.c: In function 'analyse_instr':
>> arch/powerpc/lib/sstep.c:1176:11: error: implicit declaration of function 'ppc_inst_suffix'; did you mean 'ppc_inst_swab'? [-Werror=implicit-function-declaration]
1176 | suffix = ppc_inst_suffix(instr);
| ^~~~~~~~~~~~~~~
| ppc_inst_swab
cc1: all warnings being treated as errors
vim +/ppc_inst +69 arch/powerpc/include/asm/inst.h
68
> 69 static inline bool ppc_inst_prefixed(ppc_inst x)
70 {
71 return 0;
72 }
73
74 static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
75 {
76 return ppc_inst(swab32(ppc_inst_val(x)));
77 }
78
> 79 static inline u32 ppc_inst_val(struct ppc_inst x)
80 {
81 return x.val;
82 }
83
84 static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
85 {
86 return *ptr;
87 }
88
89 static inline void ppc_inst_write(struct ppc_inst *ptr, struct ppc_inst x)
90 {
91 *ptr = x;
92 }
93
94 #endif /* __powerpc64__ */
95
96 static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
97 {
98 return !memcmp(&x, &y, sizeof(struct ppc_inst));
99 }
100
101 static inline int ppc_inst_len(struct ppc_inst x)
102 {
> 103 return (ppc_inst_prefixed(x)) ? 8 : 4;
104 }
105
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6411 bytes --]
^ permalink raw reply
* Re: [PATCH v5 18/21] powerpc64: Add prefixed instructions to instruction data type
From: Jordan Niethe @ 2020-04-06 11:13 UTC (permalink / raw)
To: Christophe Leroy
Cc: Alistair Popple, Nicholas Piggin, Balamuruhan S, linuxppc-dev,
Daniel Axtens
In-Reply-To: <4a8cf8b1-63e7-0b68-dede-48454bf5a4a7@c-s.fr>
On Mon, Apr 6, 2020 at 8:25 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 06/04/2020 à 11:52, Alistair Popple a écrit :
>
> [...]
>
> >> @@ -32,14 +76,31 @@ static inline struct ppc_inst ppc_inst_swab(struct
> >> ppc_inst x) return ppc_inst(swab32(ppc_inst_val(x)));
> >> }
> >>
> >> +static inline u32 ppc_inst_val(struct ppc_inst x)
> >> +{
> >> + return x.val;
> >> +}
> >> +
> >> static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> >> {
> >> return *ptr;
> >> }
> >>
> >> +static inline void ppc_inst_write(struct ppc_inst *ptr, struct ppc_inst x)
> >> +{
> >> + *ptr = x;
> >> +}
> >> +
> >> +#endif /* __powerpc64__ */
> >> +
> >> static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
> >> {
> >> return !memcmp(&x, &y, sizeof(struct ppc_inst));
> >> }
> >
> > Apologies for not picking this up earlier, I was hoping to get to the bottom
> > of the issue I was seeing before you sent out v5. However the above definition
> > of instruction equality does not seem correct because it does not consider the
> > case when an instruction is not prefixed - a non-prefixed instruction should be
> > considered equal if the first 32-bit opcode/value is the same. Something like:
> >
> > if (ppc_inst_prefixed(x) != ppc_inst_prefixed(y))
> > return false;
> > else if (ppc_inst_prefixed(x))
> > return !memcmp(&x, &y, sizeof(struct ppc_inst));
>
> Are we sure memcmp() is a good candidate for the comparison ? Can we do
> simpler ? Especially, I understood a prefixed instruction is a 64 bits
> properly aligned instruction, can we do a simple u64 compare ? Or is GCC
> intelligent enough to do that without calling memcmp() function which is
> heavy ?
As it is GCC does call memcmp(). I'll try to make it simpler.
>
> > else
> > return x.val == y.val;
> >
> > This was causing failures in ftrace_modify_code() as it would falsely detect
> > two non-prefixed instructions as being not equal due to differences in the suffix.
> >
>
> Christophe
^ permalink raw reply
* Re: [PATCH v5 20/21] powerpc sstep: Add support for prefixed load/stores
From: kbuild test robot @ 2020-04-06 11:29 UTC (permalink / raw)
To: Jordan Niethe
Cc: kbuild-all, Jordan Niethe, npiggin, bala24, alistair,
linuxppc-dev, dja
In-Reply-To: <20200406080936.7180-21-jniethe5@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8286 bytes --]
Hi Jordan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v5.6]
[cannot apply to powerpc/next kvm-ppc/kvm-ppc-next scottwood/next next-20200406]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Jordan-Niethe/Initial-Prefixed-Instruction-support/20200406-165215
base: 7111951b8d4973bda27ff663f2cf18b663d15b48
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from arch/powerpc/include/asm/code-patching.h:14,
from arch/powerpc/include/asm/kprobes.h:24,
from include/linux/kprobes.h:30,
from arch/powerpc/lib/sstep.c:8:
arch/powerpc/include/asm/inst.h:69:38: error: unknown type name 'ppc_inst'
69 | static inline bool ppc_inst_prefixed(ppc_inst x)
| ^~~~~~~~
arch/powerpc/include/asm/inst.h:79:19: error: redefinition of 'ppc_inst_val'
79 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~~~~~~~~~
arch/powerpc/include/asm/inst.h:21:19: note: previous definition of 'ppc_inst_val' was here
21 | static inline u32 ppc_inst_val(struct ppc_inst x)
| ^~~~~~~~~~~~
arch/powerpc/include/asm/inst.h: In function 'ppc_inst_len':
arch/powerpc/include/asm/inst.h:103:10: error: implicit declaration of function 'ppc_inst_prefixed'; did you mean 'ppc_inst_write'? [-Werror=implicit-function-declaration]
103 | return (ppc_inst_prefixed(x)) ? 8 : 4;
| ^~~~~~~~~~~~~~~~~
| ppc_inst_write
arch/powerpc/lib/sstep.c: In function 'analyse_instr':
arch/powerpc/lib/sstep.c:1215:11: error: implicit declaration of function 'ppc_inst_suffix'; did you mean 'ppc_inst_swab'? [-Werror=implicit-function-declaration]
1215 | suffix = ppc_inst_suffix(instr);
| ^~~~~~~~~~~~~~~
| ppc_inst_swab
>> arch/powerpc/lib/sstep.c:1207:41: error: unused variable 'prefix_r' [-Werror=unused-variable]
1207 | unsigned int suffixopcode, prefixtype, prefix_r;
| ^~~~~~~~
>> arch/powerpc/lib/sstep.c:1207:29: error: unused variable 'prefixtype' [-Werror=unused-variable]
1207 | unsigned int suffixopcode, prefixtype, prefix_r;
| ^~~~~~~~~~
>> arch/powerpc/lib/sstep.c:1207:15: error: unused variable 'suffixopcode' [-Werror=unused-variable]
1207 | unsigned int suffixopcode, prefixtype, prefix_r;
| ^~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/prefix_r +1207 arch/powerpc/lib/sstep.c
1191
1192 /*
1193 * Decode an instruction, and return information about it in *op
1194 * without changing *regs.
1195 * Integer arithmetic and logical instructions, branches, and barrier
1196 * instructions can be emulated just using the information in *op.
1197 *
1198 * Return value is 1 if the instruction can be emulated just by
1199 * updating *regs with the information in *op, -1 if we need the
1200 * GPRs but *regs doesn't contain the full register set, or 0
1201 * otherwise.
1202 */
1203 int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
1204 struct ppc_inst instr)
1205 {
1206 unsigned int opcode, ra, rb, rc, rd, spr, u;
> 1207 unsigned int suffixopcode, prefixtype, prefix_r;
1208 unsigned long int imm;
1209 unsigned long int val, val2;
1210 unsigned int mb, me, sh;
1211 unsigned int word, suffix;
1212 long ival;
1213
1214 word = ppc_inst_val(instr);
1215 suffix = ppc_inst_suffix(instr);
1216
1217 op->type = COMPUTE;
1218
1219 opcode = word >> 26;
1220 switch (opcode) {
1221 case 16: /* bc */
1222 op->type = BRANCH;
1223 imm = (signed short)(word & 0xfffc);
1224 if ((word & 2) == 0)
1225 imm += regs->nip;
1226 op->val = truncate_if_32bit(regs->msr, imm);
1227 if (word & 1)
1228 op->type |= SETLK;
1229 if (branch_taken(word, regs, op))
1230 op->type |= BRTAKEN;
1231 return 1;
1232 #ifdef CONFIG_PPC64
1233 case 17: /* sc */
1234 if ((word & 0xfe2) == 2)
1235 op->type = SYSCALL;
1236 else
1237 op->type = UNKNOWN;
1238 return 0;
1239 #endif
1240 case 18: /* b */
1241 op->type = BRANCH | BRTAKEN;
1242 imm = word & 0x03fffffc;
1243 if (imm & 0x02000000)
1244 imm -= 0x04000000;
1245 if ((word & 2) == 0)
1246 imm += regs->nip;
1247 op->val = truncate_if_32bit(regs->msr, imm);
1248 if (word & 1)
1249 op->type |= SETLK;
1250 return 1;
1251 case 19:
1252 switch ((word >> 1) & 0x3ff) {
1253 case 0: /* mcrf */
1254 op->type = COMPUTE + SETCC;
1255 rd = 7 - ((word >> 23) & 0x7);
1256 ra = 7 - ((word >> 18) & 0x7);
1257 rd *= 4;
1258 ra *= 4;
1259 val = (regs->ccr >> ra) & 0xf;
1260 op->ccval = (regs->ccr & ~(0xfUL << rd)) | (val << rd);
1261 return 1;
1262
1263 case 16: /* bclr */
1264 case 528: /* bcctr */
1265 op->type = BRANCH;
1266 imm = (word & 0x400)? regs->ctr: regs->link;
1267 op->val = truncate_if_32bit(regs->msr, imm);
1268 if (word & 1)
1269 op->type |= SETLK;
1270 if (branch_taken(word, regs, op))
1271 op->type |= BRTAKEN;
1272 return 1;
1273
1274 case 18: /* rfid, scary */
1275 if (regs->msr & MSR_PR)
1276 goto priv;
1277 op->type = RFI;
1278 return 0;
1279
1280 case 150: /* isync */
1281 op->type = BARRIER | BARRIER_ISYNC;
1282 return 1;
1283
1284 case 33: /* crnor */
1285 case 129: /* crandc */
1286 case 193: /* crxor */
1287 case 225: /* crnand */
1288 case 257: /* crand */
1289 case 289: /* creqv */
1290 case 417: /* crorc */
1291 case 449: /* cror */
1292 op->type = COMPUTE + SETCC;
1293 ra = (word >> 16) & 0x1f;
1294 rb = (word >> 11) & 0x1f;
1295 rd = (word >> 21) & 0x1f;
1296 ra = (regs->ccr >> (31 - ra)) & 1;
1297 rb = (regs->ccr >> (31 - rb)) & 1;
1298 val = (word >> (6 + ra * 2 + rb)) & 1;
1299 op->ccval = (regs->ccr & ~(1UL << (31 - rd))) |
1300 (val << (31 - rd));
1301 return 1;
1302 }
1303 break;
1304 case 31:
1305 switch ((word >> 1) & 0x3ff) {
1306 case 598: /* sync */
1307 op->type = BARRIER + BARRIER_SYNC;
1308 #ifdef __powerpc64__
1309 switch ((word >> 21) & 3) {
1310 case 1: /* lwsync */
1311 op->type = BARRIER + BARRIER_LWSYNC;
1312 break;
1313 case 2: /* ptesync */
1314 op->type = BARRIER + BARRIER_PTESYNC;
1315 break;
1316 }
1317 #endif
1318 return 1;
1319
1320 case 854: /* eieio */
1321 op->type = BARRIER + BARRIER_EIEIO;
1322 return 1;
1323 }
1324 break;
1325 }
1326
1327 /* Following cases refer to regs->gpr[], so we need all regs */
1328 if (!FULL_REGS(regs))
1329 return -1;
1330
1331 rd = (word >> 21) & 0x1f;
1332 ra = (word >> 16) & 0x1f;
1333 rb = (word >> 11) & 0x1f;
1334 rc = (word >> 6) & 0x1f;
1335
1336 switch (opcode) {
1337 #ifdef __powerpc64__
1338 case 2: /* tdi */
1339 if (rd & trap_compare(regs->gpr[ra], (short) word))
1340 goto trap;
1341 return 1;
1342 #endif
1343 case 3: /* twi */
1344 if (rd & trap_compare((int)regs->gpr[ra], (short) word))
1345 goto trap;
1346 return 1;
1347
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6411 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Christoph Hellwig @ 2020-04-06 11:50 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Greg Kroah-Hartman, Joerg Roedel, Robin Murphy, linux-kernel,
iommu, Aneesh Kumar K.V, linuxppc-dev, Christoph Hellwig,
Lu Baolu
In-Reply-To: <213b0c7d-f908-b4f4-466d-6240c3622cd6@ozlabs.ru>
On Fri, Apr 03, 2020 at 07:38:11PM +1100, Alexey Kardashevskiy wrote:
>
>
> On 26/03/2020 12:26, Alexey Kardashevskiy wrote:
> >
> >
> > On 25/03/2020 19:37, Christoph Hellwig wrote:
> >> On Wed, Mar 25, 2020 at 03:51:36PM +1100, Alexey Kardashevskiy wrote:
> >>>>> This is for persistent memory which you can DMA to/from but yet it does
> >>>>> not appear in the system as a normal memory and therefore requires
> >>>>> special handling anyway (O_DIRECT or DAX, I do not know the exact
> >>>>> mechanics). All other devices in the system should just run as usual,
> >>>>> i.e. use 1:1 mapping if possible.
> >>>>
> >>>> On other systems (x86 and arm) pmem as long as it is page backed does
> >>>> not require any special handling. This must be some weird way powerpc
> >>>> fucked up again, and I suspect you'll have to suffer from it.
> >>>
> >>>
> >>> It does not matter if it is backed by pages or not, the problem may also
> >>> appear if we wanted for example p2p PCI via IOMMU (between PHBs) and
> >>> MMIO might be mapped way too high in the system address space and make
> >>> 1:1 impossible.
> >>
> >> How can it be mapped too high for a direct mapping with a 64-bit DMA
> >> mask?
> >
> > The window size is limited and often it is not even sparse. It requires
> > an 8 byte entry per an IOMMU page (which is most commonly is 64k max) so
> > 1TB limit (a guest RAM size) is a quite real thing. MMIO is mapped to
> > guest physical address space outside of this 1TB (on PPC).
> >
> >
>
> I am trying now this approach on top of yours "dma-bypass.3" (it is
> "wip", needs an upper boundary check):
>
> https://github.com/aik/linux/commit/49d73c7771e3f6054804f6cfa80b4e320111662d
>
> Do you see any serious problem with this approach? Thanks!
Do you have a link to the whole branch? The github UI is unfortunately
unusable for that (or I'm missing something).
^ permalink raw reply
* Re: [PATCH v4] powerpc/powernv: add NULL check after kzalloc in opal_add_one_export
From: Markus Elfring @ 2020-04-06 12:00 UTC (permalink / raw)
To: Qiujun Huang, linuxppc-dev
Cc: LKML, Oliver O'Halloran, Paul Mackerras, Thomas Gleixner
In-Reply-To: <CAJRQjoeTYg6NnxwvXusU6fdOFOmOiceokYJZQ23-0YQo_yqZ5w@mail.gmail.com>
>>> Here needs a NULL check as kzalloc may fail returning NULL.
I find this wording potentially confusing.
* Such function calls will usually succeed to return a pointer.
* The desired memory allocation can fail.
* Please choose an imperative wording for the change description.
>>> Issue was found by coccinelle.
Please omit this line after the addition for the reference to the SmPL script.
>>> Generated by: scripts/coccinelle/null/kmerr.cocci
…
>>> Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
I wonder about this tag because of requested changes for the shown patch approach.
I recommend to add the tag “Fixes”.
>> Thanks for putting up with all the review comments :)
This seems to become challenging here.
>> But I think this should actually be two patches now.
…
> Get that, I'll separate them.
I wonder why it was not directly tried in this patch version.
>>> v3->v4:
I suggest to apply a shorter version numbering format (without an arrow).
Regards,
Markus
^ permalink raw reply
* remove set_fs calls from the exec and coredump code
From: Christoph Hellwig @ 2020-04-06 12:03 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: linux-fsdevel, linuxppc-dev, Jeremy Kerr, Arnd Bergmann,
linux-kernel
Hi all,
this series gets rid of playing with the address limit in the exec and
coredump code. Most of this was fairly trivial, the biggest changes are
those to the spufs coredump code.
^ permalink raw reply
* [PATCH 1/6] powerpc/spufs: simplify spufs core dumping
From: Christoph Hellwig @ 2020-04-06 12:03 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: linux-fsdevel, linuxppc-dev, Jeremy Kerr, Arnd Bergmann,
linux-kernel
In-Reply-To: <20200406120312.1150405-1-hch@lst.de>
Replace the coredump ->read method with a ->dump method that must call
dump_emit itself. That way we avoid a buffer allocation an messing with
set_fs() to call into code that is intended to deal with user buffers.
For the ->get case we can now use a small on-stack buffer and avoid
memory allocations as well.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/powerpc/platforms/cell/spufs/coredump.c | 87 ++----
arch/powerpc/platforms/cell/spufs/file.c | 273 ++++++++++---------
arch/powerpc/platforms/cell/spufs/spufs.h | 3 +-
3 files changed, 170 insertions(+), 193 deletions(-)
diff --git a/arch/powerpc/platforms/cell/spufs/coredump.c b/arch/powerpc/platforms/cell/spufs/coredump.c
index 8b3296b62f65..3b75e8f60609 100644
--- a/arch/powerpc/platforms/cell/spufs/coredump.c
+++ b/arch/powerpc/platforms/cell/spufs/coredump.c
@@ -21,22 +21,6 @@
#include "spufs.h"
-static ssize_t do_coredump_read(int num, struct spu_context *ctx, void *buffer,
- size_t size, loff_t *off)
-{
- u64 data;
- int ret;
-
- if (spufs_coredump_read[num].read)
- return spufs_coredump_read[num].read(ctx, buffer, size, off);
-
- data = spufs_coredump_read[num].get(ctx);
- ret = snprintf(buffer, size, "0x%.16llx", data);
- if (ret >= size)
- return size;
- return ++ret; /* count trailing NULL */
-}
-
static int spufs_ctx_note_size(struct spu_context *ctx, int dfd)
{
int i, sz, total = 0;
@@ -118,58 +102,43 @@ int spufs_coredump_extra_notes_size(void)
static int spufs_arch_write_note(struct spu_context *ctx, int i,
struct coredump_params *cprm, int dfd)
{
- loff_t pos = 0;
- int sz, rc, total = 0;
- const int bufsz = PAGE_SIZE;
- char *name;
- char fullname[80], *buf;
+ size_t sz = spufs_coredump_read[i].size;
+ char fullname[80];
struct elf_note en;
- size_t skip;
-
- buf = (void *)get_zeroed_page(GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
+ size_t ret;
- name = spufs_coredump_read[i].name;
- sz = spufs_coredump_read[i].size;
-
- sprintf(fullname, "SPU/%d/%s", dfd, name);
+ sprintf(fullname, "SPU/%d/%s", dfd, spufs_coredump_read[i].name);
en.n_namesz = strlen(fullname) + 1;
en.n_descsz = sz;
en.n_type = NT_SPU;
if (!dump_emit(cprm, &en, sizeof(en)))
- goto Eio;
-
+ return -EIO;
if (!dump_emit(cprm, fullname, en.n_namesz))
- goto Eio;
-
+ return -EIO;
if (!dump_align(cprm, 4))
- goto Eio;
-
- do {
- rc = do_coredump_read(i, ctx, buf, bufsz, &pos);
- if (rc > 0) {
- if (!dump_emit(cprm, buf, rc))
- goto Eio;
- total += rc;
- }
- } while (rc == bufsz && total < sz);
-
- if (rc < 0)
- goto out;
-
- skip = roundup(cprm->pos - total + sz, 4) - cprm->pos;
- if (!dump_skip(cprm, skip))
- goto Eio;
-
- rc = 0;
-out:
- free_page((unsigned long)buf);
- return rc;
-Eio:
- free_page((unsigned long)buf);
- return -EIO;
+ return -EIO;
+
+ if (spufs_coredump_read[i].dump) {
+ ret = spufs_coredump_read[i].dump(ctx, cprm);
+ if (ret < 0)
+ return ret;
+ } else {
+ char buf[32];
+
+ ret = snprintf(buf, sizeof(buf), "0x%.16llx",
+ spufs_coredump_read[i].get(ctx));
+ if (ret >= sizeof(buf))
+ return sizeof(buf);
+
+ /* count trailing the NULL: */
+ if (!dump_emit(cprm, buf, ret + 1))
+ return -EIO;
+ }
+
+ if (!dump_skip(cprm, roundup(cprm->pos - ret + sz, 4) - cprm->pos))
+ return -EIO;
+ return 0;
}
int spufs_coredump_extra_notes_write(struct coredump_params *cprm)
diff --git a/arch/powerpc/platforms/cell/spufs/file.c b/arch/powerpc/platforms/cell/spufs/file.c
index c0f950a3f4e1..d9574c4b1347 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -9,6 +9,7 @@
#undef DEBUG
+#include <linux/coredump.h>
#include <linux/fs.h>
#include <linux/ioctl.h>
#include <linux/export.h>
@@ -129,6 +130,14 @@ static ssize_t spufs_attr_write(struct file *file, const char __user *buf,
return ret;
}
+static ssize_t spufs_dump_emit(struct coredump_params *cprm, void *buf,
+ size_t size)
+{
+ if (!dump_emit(cprm, buf, size))
+ return -EIO;
+ return size;
+}
+
#define DEFINE_SPUFS_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt) \
static int __fops ## _open(struct inode *inode, struct file *file) \
{ \
@@ -172,12 +181,9 @@ spufs_mem_release(struct inode *inode, struct file *file)
}
static ssize_t
-__spufs_mem_read(struct spu_context *ctx, char __user *buffer,
- size_t size, loff_t *pos)
+spufs_mem_dump(struct spu_context *ctx, struct coredump_params *cprm)
{
- char *local_store = ctx->ops->get_ls(ctx);
- return simple_read_from_buffer(buffer, size, pos, local_store,
- LS_SIZE);
+ return spufs_dump_emit(cprm, ctx->ops->get_ls(ctx), LS_SIZE);
}
static ssize_t
@@ -190,7 +196,8 @@ spufs_mem_read(struct file *file, char __user *buffer,
ret = spu_acquire(ctx);
if (ret)
return ret;
- ret = __spufs_mem_read(ctx, buffer, size, pos);
+ ret = simple_read_from_buffer(buffer, size, pos, ctx->ops->get_ls(ctx),
+ LS_SIZE);
spu_release(ctx);
return ret;
@@ -459,12 +466,10 @@ spufs_regs_open(struct inode *inode, struct file *file)
}
static ssize_t
-__spufs_regs_read(struct spu_context *ctx, char __user *buffer,
- size_t size, loff_t *pos)
+spufs_regs_dump(struct spu_context *ctx, struct coredump_params *cprm)
{
- struct spu_lscsa *lscsa = ctx->csa.lscsa;
- return simple_read_from_buffer(buffer, size, pos,
- lscsa->gprs, sizeof lscsa->gprs);
+ return spufs_dump_emit(cprm, ctx->csa.lscsa->gprs,
+ sizeof(ctx->csa.lscsa->gprs));
}
static ssize_t
@@ -482,7 +487,8 @@ spufs_regs_read(struct file *file, char __user *buffer,
ret = spu_acquire_saved(ctx);
if (ret)
return ret;
- ret = __spufs_regs_read(ctx, buffer, size, pos);
+ ret = simple_read_from_buffer(buffer, size, pos, ctx->csa.lscsa->gprs,
+ sizeof(ctx->csa.lscsa->gprs));
spu_release_saved(ctx);
return ret;
}
@@ -517,12 +523,10 @@ static const struct file_operations spufs_regs_fops = {
};
static ssize_t
-__spufs_fpcr_read(struct spu_context *ctx, char __user * buffer,
- size_t size, loff_t * pos)
+spufs_fpcr_dump(struct spu_context *ctx, struct coredump_params *cprm)
{
- struct spu_lscsa *lscsa = ctx->csa.lscsa;
- return simple_read_from_buffer(buffer, size, pos,
- &lscsa->fpcr, sizeof(lscsa->fpcr));
+ return spufs_dump_emit(cprm, &ctx->csa.lscsa->fpcr,
+ sizeof(ctx->csa.lscsa->fpcr));
}
static ssize_t
@@ -535,7 +539,8 @@ spufs_fpcr_read(struct file *file, char __user * buffer,
ret = spu_acquire_saved(ctx);
if (ret)
return ret;
- ret = __spufs_fpcr_read(ctx, buffer, size, pos);
+ ret = simple_read_from_buffer(buffer, size, pos, &ctx->csa.lscsa->fpcr,
+ sizeof(ctx->csa.lscsa->fpcr));
spu_release_saved(ctx);
return ret;
}
@@ -967,28 +972,26 @@ spufs_signal1_release(struct inode *inode, struct file *file)
return 0;
}
-static ssize_t __spufs_signal1_read(struct spu_context *ctx, char __user *buf,
- size_t len, loff_t *pos)
+static ssize_t spufs_signal1_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
{
- int ret = 0;
- u32 data;
+ if (!ctx->csa.spu_chnlcnt_RW[3])
+ return 0;
+ return spufs_dump_emit(cprm, &ctx->csa.spu_chnldata_RW[3],
+ sizeof(ctx->csa.spu_chnldata_RW[3]));
+}
- if (len < 4)
+static ssize_t __spufs_signal1_read(struct spu_context *ctx, char __user *buf,
+ size_t len)
+{
+ if (len < sizeof(ctx->csa.spu_chnldata_RW[3]))
return -EINVAL;
-
- if (ctx->csa.spu_chnlcnt_RW[3]) {
- data = ctx->csa.spu_chnldata_RW[3];
- ret = 4;
- }
-
- if (!ret)
- goto out;
-
- if (copy_to_user(buf, &data, 4))
+ if (!ctx->csa.spu_chnlcnt_RW[3])
+ return 0;
+ if (copy_to_user(buf, &ctx->csa.spu_chnldata_RW[3],
+ sizeof(ctx->csa.spu_chnldata_RW[3])))
return -EFAULT;
-
-out:
- return ret;
+ return sizeof(ctx->csa.spu_chnldata_RW[3]);
}
static ssize_t spufs_signal1_read(struct file *file, char __user *buf,
@@ -1000,7 +1003,7 @@ static ssize_t spufs_signal1_read(struct file *file, char __user *buf,
ret = spu_acquire_saved(ctx);
if (ret)
return ret;
- ret = __spufs_signal1_read(ctx, buf, len, pos);
+ ret = __spufs_signal1_read(ctx, buf, len);
spu_release_saved(ctx);
return ret;
@@ -1104,28 +1107,26 @@ spufs_signal2_release(struct inode *inode, struct file *file)
return 0;
}
-static ssize_t __spufs_signal2_read(struct spu_context *ctx, char __user *buf,
- size_t len, loff_t *pos)
+static ssize_t spufs_signal2_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
{
- int ret = 0;
- u32 data;
+ if (!ctx->csa.spu_chnlcnt_RW[4])
+ return 0;
+ return spufs_dump_emit(cprm, &ctx->csa.spu_chnldata_RW[4],
+ sizeof(ctx->csa.spu_chnldata_RW[4]));
+}
- if (len < 4)
+static ssize_t __spufs_signal2_read(struct spu_context *ctx, char __user *buf,
+ size_t len)
+{
+ if (len < sizeof(ctx->csa.spu_chnldata_RW[4]))
return -EINVAL;
-
- if (ctx->csa.spu_chnlcnt_RW[4]) {
- data = ctx->csa.spu_chnldata_RW[4];
- ret = 4;
- }
-
- if (!ret)
- goto out;
-
- if (copy_to_user(buf, &data, 4))
+ if (!ctx->csa.spu_chnlcnt_RW[4])
+ return 0;
+ if (copy_to_user(buf, &ctx->csa.spu_chnldata_RW[4],
+ sizeof(ctx->csa.spu_chnldata_RW[4])))
return -EFAULT;
-
-out:
- return ret;
+ return sizeof(ctx->csa.spu_chnldata_RW[4]);
}
static ssize_t spufs_signal2_read(struct file *file, char __user *buf,
@@ -1137,7 +1138,7 @@ static ssize_t spufs_signal2_read(struct file *file, char __user *buf,
ret = spu_acquire_saved(ctx);
if (ret)
return ret;
- ret = __spufs_signal2_read(ctx, buf, len, pos);
+ ret = __spufs_signal2_read(ctx, buf, len);
spu_release_saved(ctx);
return ret;
@@ -1961,18 +1962,13 @@ static const struct file_operations spufs_caps_fops = {
.release = single_release,
};
-static ssize_t __spufs_mbox_info_read(struct spu_context *ctx,
- char __user *buf, size_t len, loff_t *pos)
+static ssize_t spufs_mbox_info_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
{
- u32 data;
-
- /* EOF if there's no entry in the mbox */
if (!(ctx->csa.prob.mb_stat_R & 0x0000ff))
return 0;
-
- data = ctx->csa.prob.pu_mb_R;
-
- return simple_read_from_buffer(buf, len, pos, &data, sizeof data);
+ return spufs_dump_emit(cprm, &ctx->csa.prob.pu_mb_R,
+ sizeof(ctx->csa.prob.pu_mb_R));
}
static ssize_t spufs_mbox_info_read(struct file *file, char __user *buf,
@@ -1988,7 +1984,12 @@ static ssize_t spufs_mbox_info_read(struct file *file, char __user *buf,
if (ret)
return ret;
spin_lock(&ctx->csa.register_lock);
- ret = __spufs_mbox_info_read(ctx, buf, len, pos);
+ /* EOF if there's no entry in the mbox */
+ if (ctx->csa.prob.mb_stat_R & 0x0000ff) {
+ ret = simple_read_from_buffer(buf, len, pos,
+ &ctx->csa.prob.pu_mb_R,
+ sizeof(ctx->csa.prob.pu_mb_R));
+ }
spin_unlock(&ctx->csa.register_lock);
spu_release_saved(ctx);
@@ -2001,18 +2002,13 @@ static const struct file_operations spufs_mbox_info_fops = {
.llseek = generic_file_llseek,
};
-static ssize_t __spufs_ibox_info_read(struct spu_context *ctx,
- char __user *buf, size_t len, loff_t *pos)
+static ssize_t spufs_ibox_info_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
{
- u32 data;
-
- /* EOF if there's no entry in the ibox */
if (!(ctx->csa.prob.mb_stat_R & 0xff0000))
return 0;
-
- data = ctx->csa.priv2.puint_mb_R;
-
- return simple_read_from_buffer(buf, len, pos, &data, sizeof data);
+ return spufs_dump_emit(cprm, &ctx->csa.priv2.puint_mb_R,
+ sizeof(ctx->csa.priv2.puint_mb_R));
}
static ssize_t spufs_ibox_info_read(struct file *file, char __user *buf,
@@ -2028,7 +2024,12 @@ static ssize_t spufs_ibox_info_read(struct file *file, char __user *buf,
if (ret)
return ret;
spin_lock(&ctx->csa.register_lock);
- ret = __spufs_ibox_info_read(ctx, buf, len, pos);
+ /* EOF if there's no entry in the ibox */
+ if (ctx->csa.prob.mb_stat_R & 0xff0000) {
+ ret = simple_read_from_buffer(buf, len, pos,
+ &ctx->csa.priv2.puint_mb_R,
+ sizeof(ctx->csa.priv2.puint_mb_R));
+ }
spin_unlock(&ctx->csa.register_lock);
spu_release_saved(ctx);
@@ -2041,21 +2042,16 @@ static const struct file_operations spufs_ibox_info_fops = {
.llseek = generic_file_llseek,
};
-static ssize_t __spufs_wbox_info_read(struct spu_context *ctx,
- char __user *buf, size_t len, loff_t *pos)
+static size_t spufs_wbox_info_cnt(struct spu_context *ctx)
{
- int i, cnt;
- u32 data[4];
- u32 wbox_stat;
-
- wbox_stat = ctx->csa.prob.mb_stat_R;
- cnt = 4 - ((wbox_stat & 0x00ff00) >> 8);
- for (i = 0; i < cnt; i++) {
- data[i] = ctx->csa.spu_mailbox_data[i];
- }
+ return (4 - ((ctx->csa.prob.mb_stat_R & 0x00ff00) >> 8)) * sizeof(u32);
+}
- return simple_read_from_buffer(buf, len, pos, &data,
- cnt * sizeof(u32));
+static ssize_t spufs_wbox_info_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
+{
+ return spufs_dump_emit(cprm, &ctx->csa.spu_mailbox_data,
+ spufs_wbox_info_cnt(ctx));
}
static ssize_t spufs_wbox_info_read(struct file *file, char __user *buf,
@@ -2071,7 +2067,8 @@ static ssize_t spufs_wbox_info_read(struct file *file, char __user *buf,
if (ret)
return ret;
spin_lock(&ctx->csa.register_lock);
- ret = __spufs_wbox_info_read(ctx, buf, len, pos);
+ ret = simple_read_from_buffer(buf, len, pos, &ctx->csa.spu_mailbox_data,
+ spufs_wbox_info_cnt(ctx));
spin_unlock(&ctx->csa.register_lock);
spu_release_saved(ctx);
@@ -2084,36 +2081,42 @@ static const struct file_operations spufs_wbox_info_fops = {
.llseek = generic_file_llseek,
};
-static ssize_t __spufs_dma_info_read(struct spu_context *ctx,
- char __user *buf, size_t len, loff_t *pos)
+static void __spufs_dma_info_read(struct spu_context *ctx,
+ struct spu_dma_info *info)
{
- struct spu_dma_info info;
- struct mfc_cq_sr *qp, *spuqp;
int i;
- info.dma_info_type = ctx->csa.priv2.spu_tag_status_query_RW;
- info.dma_info_mask = ctx->csa.lscsa->tag_mask.slot[0];
- info.dma_info_status = ctx->csa.spu_chnldata_RW[24];
- info.dma_info_stall_and_notify = ctx->csa.spu_chnldata_RW[25];
- info.dma_info_atomic_command_status = ctx->csa.spu_chnldata_RW[27];
+ info->dma_info_type = ctx->csa.priv2.spu_tag_status_query_RW;
+ info->dma_info_mask = ctx->csa.lscsa->tag_mask.slot[0];
+ info->dma_info_status = ctx->csa.spu_chnldata_RW[24];
+ info->dma_info_stall_and_notify = ctx->csa.spu_chnldata_RW[25];
+ info->dma_info_atomic_command_status = ctx->csa.spu_chnldata_RW[27];
+
for (i = 0; i < 16; i++) {
- qp = &info.dma_info_command_data[i];
- spuqp = &ctx->csa.priv2.spuq[i];
+ struct mfc_cq_sr *qp = &info->dma_info_command_data[i];
+ struct mfc_cq_sr *spuqp = &ctx->csa.priv2.spuq[i];
qp->mfc_cq_data0_RW = spuqp->mfc_cq_data0_RW;
qp->mfc_cq_data1_RW = spuqp->mfc_cq_data1_RW;
qp->mfc_cq_data2_RW = spuqp->mfc_cq_data2_RW;
qp->mfc_cq_data3_RW = spuqp->mfc_cq_data3_RW;
}
+}
+
+static ssize_t spufs_dma_info_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
+{
+ struct spu_dma_info info;
- return simple_read_from_buffer(buf, len, pos, &info,
- sizeof info);
+ __spufs_dma_info_read(ctx, &info);
+ return spufs_dump_emit(cprm, &info, sizeof(info));
}
static ssize_t spufs_dma_info_read(struct file *file, char __user *buf,
size_t len, loff_t *pos)
{
struct spu_context *ctx = file->private_data;
+ struct spu_dma_info info;
int ret;
if (!access_ok(buf, len))
@@ -2123,7 +2126,8 @@ static ssize_t spufs_dma_info_read(struct file *file, char __user *buf,
if (ret)
return ret;
spin_lock(&ctx->csa.register_lock);
- ret = __spufs_dma_info_read(ctx, buf, len, pos);
+ __spufs_dma_info_read(ctx, &info);
+ ret = simple_read_from_buffer(buf, len, pos, &info, sizeof(info));
spin_unlock(&ctx->csa.register_lock);
spu_release_saved(ctx);
@@ -2136,48 +2140,53 @@ static const struct file_operations spufs_dma_info_fops = {
.llseek = no_llseek,
};
-static ssize_t __spufs_proxydma_info_read(struct spu_context *ctx,
- char __user *buf, size_t len, loff_t *pos)
+static void __spufs_proxydma_info_read(struct spu_context *ctx,
+ struct spu_proxydma_info *info)
{
- struct spu_proxydma_info info;
- struct mfc_cq_sr *qp, *puqp;
- int ret = sizeof info;
int i;
- if (len < ret)
- return -EINVAL;
-
- if (!access_ok(buf, len))
- return -EFAULT;
+ info->proxydma_info_type = ctx->csa.prob.dma_querytype_RW;
+ info->proxydma_info_mask = ctx->csa.prob.dma_querymask_RW;
+ info->proxydma_info_status = ctx->csa.prob.dma_tagstatus_R;
- info.proxydma_info_type = ctx->csa.prob.dma_querytype_RW;
- info.proxydma_info_mask = ctx->csa.prob.dma_querymask_RW;
- info.proxydma_info_status = ctx->csa.prob.dma_tagstatus_R;
for (i = 0; i < 8; i++) {
- qp = &info.proxydma_info_command_data[i];
- puqp = &ctx->csa.priv2.puq[i];
+ struct mfc_cq_sr *qp = &info->proxydma_info_command_data[i];
+ struct mfc_cq_sr *puqp = &ctx->csa.priv2.puq[i];
qp->mfc_cq_data0_RW = puqp->mfc_cq_data0_RW;
qp->mfc_cq_data1_RW = puqp->mfc_cq_data1_RW;
qp->mfc_cq_data2_RW = puqp->mfc_cq_data2_RW;
qp->mfc_cq_data3_RW = puqp->mfc_cq_data3_RW;
}
+}
+
+static ssize_t spufs_proxydma_info_dump(struct spu_context *ctx,
+ struct coredump_params *cprm)
+{
+ struct spu_proxydma_info info;
- return simple_read_from_buffer(buf, len, pos, &info,
- sizeof info);
+ __spufs_proxydma_info_read(ctx, &info);
+ return spufs_dump_emit(cprm, &info, sizeof(info));
}
static ssize_t spufs_proxydma_info_read(struct file *file, char __user *buf,
size_t len, loff_t *pos)
{
struct spu_context *ctx = file->private_data;
+ struct spu_proxydma_info info;
int ret;
+ if (len < sizeof(info))
+ return -EINVAL;
+ if (!access_ok(buf, len))
+ return -EFAULT;
+
ret = spu_acquire_saved(ctx);
if (ret)
return ret;
spin_lock(&ctx->csa.register_lock);
- ret = __spufs_proxydma_info_read(ctx, buf, len, pos);
+ __spufs_proxydma_info_read(ctx, &info);
+ ret = simple_read_from_buffer(buf, len, pos, &info, sizeof(info));
spin_unlock(&ctx->csa.register_lock);
spu_release_saved(ctx);
@@ -2625,23 +2634,23 @@ const struct spufs_tree_descr spufs_dir_debug_contents[] = {
};
const struct spufs_coredump_reader spufs_coredump_read[] = {
- { "regs", __spufs_regs_read, NULL, sizeof(struct spu_reg128[128])},
- { "fpcr", __spufs_fpcr_read, NULL, sizeof(struct spu_reg128) },
+ { "regs", spufs_regs_dump, NULL, sizeof(struct spu_reg128[128])},
+ { "fpcr", spufs_fpcr_dump, NULL, sizeof(struct spu_reg128) },
{ "lslr", NULL, spufs_lslr_get, 19 },
{ "decr", NULL, spufs_decr_get, 19 },
{ "decr_status", NULL, spufs_decr_status_get, 19 },
- { "mem", __spufs_mem_read, NULL, LS_SIZE, },
- { "signal1", __spufs_signal1_read, NULL, sizeof(u32) },
+ { "mem", spufs_mem_dump, NULL, LS_SIZE, },
+ { "signal1", spufs_signal1_dump, NULL, sizeof(u32) },
{ "signal1_type", NULL, spufs_signal1_type_get, 19 },
- { "signal2", __spufs_signal2_read, NULL, sizeof(u32) },
+ { "signal2", spufs_signal2_dump, NULL, sizeof(u32) },
{ "signal2_type", NULL, spufs_signal2_type_get, 19 },
{ "event_mask", NULL, spufs_event_mask_get, 19 },
{ "event_status", NULL, spufs_event_status_get, 19 },
- { "mbox_info", __spufs_mbox_info_read, NULL, sizeof(u32) },
- { "ibox_info", __spufs_ibox_info_read, NULL, sizeof(u32) },
- { "wbox_info", __spufs_wbox_info_read, NULL, 4 * sizeof(u32)},
- { "dma_info", __spufs_dma_info_read, NULL, sizeof(struct spu_dma_info)},
- { "proxydma_info", __spufs_proxydma_info_read,
+ { "mbox_info", spufs_mbox_info_dump, NULL, sizeof(u32) },
+ { "ibox_info", spufs_ibox_info_dump, NULL, sizeof(u32) },
+ { "wbox_info", spufs_wbox_info_dump, NULL, 4 * sizeof(u32)},
+ { "dma_info", spufs_dma_info_dump, NULL, sizeof(struct spu_dma_info)},
+ { "proxydma_info", spufs_proxydma_info_dump,
NULL, sizeof(struct spu_proxydma_info)},
{ "object-id", NULL, spufs_object_id_get, 19 },
{ "npc", NULL, spufs_npc_get, 19 },
diff --git a/arch/powerpc/platforms/cell/spufs/spufs.h b/arch/powerpc/platforms/cell/spufs/spufs.h
index 413c89afe112..1ba4d884febf 100644
--- a/arch/powerpc/platforms/cell/spufs/spufs.h
+++ b/arch/powerpc/platforms/cell/spufs/spufs.h
@@ -337,8 +337,7 @@ void spufs_dma_callback(struct spu *spu, int type);
extern struct spu_coredump_calls spufs_coredump_calls;
struct spufs_coredump_reader {
char *name;
- ssize_t (*read)(struct spu_context *ctx,
- char __user *buffer, size_t size, loff_t *pos);
+ ssize_t (*dump)(struct spu_context *ctx, struct coredump_params *cprm);
u64 (*get)(struct spu_context *ctx);
size_t size;
};
--
2.25.1
^ permalink raw reply related
* [PATCH 2/6] binfmt_elf: open code copy_siginfo_to_user to kernelspace buffer
From: Christoph Hellwig @ 2020-04-06 12:03 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: linux-fsdevel, linuxppc-dev, Jeremy Kerr, Arnd Bergmann,
linux-kernel
In-Reply-To: <20200406120312.1150405-1-hch@lst.de>
Instead of messing with the address limit just open code the trivial
memcpy + memset logic.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/binfmt_elf.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index f4713ea76e82..d744ce9a4b52 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1556,10 +1556,9 @@ static void fill_auxv_note(struct memelfnote *note, struct mm_struct *mm)
static void fill_siginfo_note(struct memelfnote *note, user_siginfo_t *csigdata,
const kernel_siginfo_t *siginfo)
{
- mm_segment_t old_fs = get_fs();
- set_fs(KERNEL_DS);
- copy_siginfo_to_user((user_siginfo_t __user *) csigdata, siginfo);
- set_fs(old_fs);
+ memcpy(csigdata, siginfo, sizeof(struct kernel_siginfo));
+ memset((char *)csigdata + sizeof(struct kernel_siginfo), 0,
+ SI_EXPANSION_SIZE);
fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata);
}
--
2.25.1
^ permalink raw reply related
* [PATCH 3/6] binfmt_elf: remove the set_fs(KERNEL_DS) in elf_core_dump
From: Christoph Hellwig @ 2020-04-06 12:03 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: linux-fsdevel, linuxppc-dev, Jeremy Kerr, Arnd Bergmann,
linux-kernel
In-Reply-To: <20200406120312.1150405-1-hch@lst.de>
There is no logic in elf_core_dump itself that uses uaccess routines
on kernel pointers, the file writes are nicely encapsulated in dump_emit
which does its own set_fs.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/binfmt_elf.c | 40 +++++++++++++---------------------------
1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index d744ce9a4b52..ef9f68bab7be 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1355,7 +1355,6 @@ static unsigned long vma_dump_size(struct vm_area_struct *vma,
vma->vm_pgoff == 0 && (vma->vm_flags & VM_READ)) {
u32 __user *header = (u32 __user *) vma->vm_start;
u32 word;
- mm_segment_t fs = get_fs();
/*
* Doing it this way gets the constant folded by GCC.
*/
@@ -1368,14 +1367,8 @@ static unsigned long vma_dump_size(struct vm_area_struct *vma,
magic.elfmag[EI_MAG1] = ELFMAG1;
magic.elfmag[EI_MAG2] = ELFMAG2;
magic.elfmag[EI_MAG3] = ELFMAG3;
- /*
- * Switch to the user "segment" for get_user(),
- * then put back what elf_core_dump() had in place.
- */
- set_fs(USER_DS);
if (unlikely(get_user(word, header)))
word = 0;
- set_fs(fs);
if (word == magic.cmp)
return PAGE_SIZE;
}
@@ -2185,7 +2178,6 @@ static void fill_extnum_info(struct elfhdr *elf, struct elf_shdr *shdr4extnum,
static int elf_core_dump(struct coredump_params *cprm)
{
int has_dumped = 0;
- mm_segment_t fs;
int segs, i;
size_t vma_data_size = 0;
struct vm_area_struct *vma, *gate_vma;
@@ -2238,9 +2230,6 @@ static int elf_core_dump(struct coredump_params *cprm)
has_dumped = 1;
- fs = get_fs();
- set_fs(KERNEL_DS);
-
offset += sizeof(elf); /* Elf header */
offset += segs * sizeof(struct elf_phdr); /* Program headers */
@@ -2252,7 +2241,7 @@ static int elf_core_dump(struct coredump_params *cprm)
phdr4note = kmalloc(sizeof(*phdr4note), GFP_KERNEL);
if (!phdr4note)
- goto end_coredump;
+ goto cleanup;
fill_elf_note_phdr(phdr4note, sz, offset);
offset += sz;
@@ -2267,7 +2256,7 @@ static int elf_core_dump(struct coredump_params *cprm)
vma_filesz = kvmalloc(array_size(sizeof(*vma_filesz), (segs - 1)),
GFP_KERNEL);
if (!vma_filesz)
- goto end_coredump;
+ goto cleanup;
for (i = 0, vma = first_vma(current, gate_vma); vma != NULL;
vma = next_vma(vma, gate_vma)) {
@@ -2285,17 +2274,17 @@ static int elf_core_dump(struct coredump_params *cprm)
if (e_phnum == PN_XNUM) {
shdr4extnum = kmalloc(sizeof(*shdr4extnum), GFP_KERNEL);
if (!shdr4extnum)
- goto end_coredump;
+ goto cleanup;
fill_extnum_info(&elf, shdr4extnum, e_shoff, segs);
}
offset = dataoff;
if (!dump_emit(cprm, &elf, sizeof(elf)))
- goto end_coredump;
+ goto cleanup;
if (!dump_emit(cprm, phdr4note, sizeof(*phdr4note)))
- goto end_coredump;
+ goto cleanup;
/* Write program headers for segments dump */
for (i = 0, vma = first_vma(current, gate_vma); vma != NULL;
@@ -2317,22 +2306,22 @@ static int elf_core_dump(struct coredump_params *cprm)
phdr.p_align = ELF_EXEC_PAGESIZE;
if (!dump_emit(cprm, &phdr, sizeof(phdr)))
- goto end_coredump;
+ goto cleanup;
}
if (!elf_core_write_extra_phdrs(cprm, offset))
- goto end_coredump;
+ goto cleanup;
/* write out the notes section */
if (!write_note_info(&info, cprm))
- goto end_coredump;
+ goto cleanup;
if (elf_coredump_extra_notes_write(cprm))
- goto end_coredump;
+ goto cleanup;
/* Align to page */
if (!dump_skip(cprm, dataoff - cprm->pos))
- goto end_coredump;
+ goto cleanup;
for (i = 0, vma = first_vma(current, gate_vma); vma != NULL;
vma = next_vma(vma, gate_vma)) {
@@ -2354,22 +2343,19 @@ static int elf_core_dump(struct coredump_params *cprm)
} else
stop = !dump_skip(cprm, PAGE_SIZE);
if (stop)
- goto end_coredump;
+ goto cleanup;
}
}
dump_truncate(cprm);
if (!elf_core_write_extra_data(cprm))
- goto end_coredump;
+ goto cleanup;
if (e_phnum == PN_XNUM) {
if (!dump_emit(cprm, shdr4extnum, sizeof(*shdr4extnum)))
- goto end_coredump;
+ goto cleanup;
}
-end_coredump:
- set_fs(fs);
-
cleanup:
free_note_info(&info);
kfree(shdr4extnum);
--
2.25.1
^ permalink raw reply related
* [PATCH 4/6] binfmt_elf_fdpic: remove the set_fs(KERNEL_DS) in elf_fdpic_core_dump
From: Christoph Hellwig @ 2020-04-06 12:03 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: linux-fsdevel, linuxppc-dev, Jeremy Kerr, Arnd Bergmann,
linux-kernel
In-Reply-To: <20200406120312.1150405-1-hch@lst.de>
There is no logic in elf_fdpic_core_dump itself that uses uaccess
routines on kernel pointers, the file writes are nicely encapsulated in
dump_emit which does its own set_fs.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/binfmt_elf_fdpic.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 240f66663543..c62c17a5c34a 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -1549,7 +1549,6 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
{
#define NUM_NOTES 6
int has_dumped = 0;
- mm_segment_t fs;
int segs;
int i;
struct vm_area_struct *vma;
@@ -1678,9 +1677,6 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
"LINUX", ELF_CORE_XFPREG_TYPE, sizeof(*xfpu), xfpu);
#endif
- fs = get_fs();
- set_fs(KERNEL_DS);
-
offset += sizeof(*elf); /* Elf header */
offset += segs * sizeof(struct elf_phdr); /* Program headers */
@@ -1695,7 +1691,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
phdr4note = kmalloc(sizeof(*phdr4note), GFP_KERNEL);
if (!phdr4note)
- goto end_coredump;
+ goto cleanup;
fill_elf_note_phdr(phdr4note, sz, offset);
offset += sz;
@@ -1711,17 +1707,17 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
if (e_phnum == PN_XNUM) {
shdr4extnum = kmalloc(sizeof(*shdr4extnum), GFP_KERNEL);
if (!shdr4extnum)
- goto end_coredump;
+ goto cleanup;
fill_extnum_info(elf, shdr4extnum, e_shoff, segs);
}
offset = dataoff;
if (!dump_emit(cprm, elf, sizeof(*elf)))
- goto end_coredump;
+ goto cleanup;
if (!dump_emit(cprm, phdr4note, sizeof(*phdr4note)))
- goto end_coredump;
+ goto cleanup;
/* write program headers for segments dump */
for (vma = current->mm->mmap; vma; vma = vma->vm_next) {
@@ -1745,16 +1741,16 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
phdr.p_align = ELF_EXEC_PAGESIZE;
if (!dump_emit(cprm, &phdr, sizeof(phdr)))
- goto end_coredump;
+ goto cleanup;
}
if (!elf_core_write_extra_phdrs(cprm, offset))
- goto end_coredump;
+ goto cleanup;
/* write out the notes section */
for (i = 0; i < numnote; i++)
if (!writenote(notes + i, cprm))
- goto end_coredump;
+ goto cleanup;
/* write out the thread status notes section */
list_for_each(t, &thread_list) {
@@ -1763,21 +1759,21 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
for (i = 0; i < tmp->num_notes; i++)
if (!writenote(&tmp->notes[i], cprm))
- goto end_coredump;
+ goto cleanup;
}
if (!dump_skip(cprm, dataoff - cprm->pos))
- goto end_coredump;
+ goto cleanup;
if (!elf_fdpic_dump_segments(cprm))
- goto end_coredump;
+ goto cleanup;
if (!elf_core_write_extra_data(cprm))
- goto end_coredump;
+ goto cleanup;
if (e_phnum == PN_XNUM) {
if (!dump_emit(cprm, shdr4extnum, sizeof(*shdr4extnum)))
- goto end_coredump;
+ goto cleanup;
}
if (cprm->file->f_pos != offset) {
@@ -1787,9 +1783,6 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
cprm->file->f_pos, offset);
}
-end_coredump:
- set_fs(fs);
-
cleanup:
while (!list_empty(&thread_list)) {
struct list_head *tmp = thread_list.next;
--
2.25.1
^ permalink raw reply related
* [PATCH 5/6] exec: simplify the copy_strings_kernel calling convention
From: Christoph Hellwig @ 2020-04-06 12:03 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: linux-fsdevel, linuxppc-dev, Jeremy Kerr, Arnd Bergmann,
linux-kernel
In-Reply-To: <20200406120312.1150405-1-hch@lst.de>
copy_strings_kernel is always used with a single argument,
adjust the calling convention to that.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/binfmt_em86.c | 6 +++---
fs/binfmt_misc.c | 4 ++--
fs/binfmt_script.c | 6 +++---
fs/exec.c | 13 ++++++-------
include/linux/binfmts.h | 3 +--
5 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 466497860c62..f33fa668c91f 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -68,15 +68,15 @@ static int load_em86(struct linux_binprm *bprm)
* user environment and arguments are stored.
*/
remove_arg_zero(bprm);
- retval = copy_strings_kernel(1, &bprm->filename, bprm);
+ retval = copy_string_kernel(bprm->filename, bprm);
if (retval < 0) return retval;
bprm->argc++;
if (i_arg) {
- retval = copy_strings_kernel(1, &i_arg, bprm);
+ retval = copy_string_kernel(i_arg, bprm);
if (retval < 0) return retval;
bprm->argc++;
}
- retval = copy_strings_kernel(1, &i_name, bprm);
+ retval = copy_string_kernel(i_name, bprm);
if (retval < 0) return retval;
bprm->argc++;
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index cdb45829354d..b15257d8ff5e 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -190,13 +190,13 @@ static int load_misc_binary(struct linux_binprm *bprm)
bprm->file = NULL;
}
/* make argv[1] be the path to the binary */
- retval = copy_strings_kernel(1, &bprm->interp, bprm);
+ retval = copy_string_kernel(bprm->interp, bprm);
if (retval < 0)
goto error;
bprm->argc++;
/* add the interp as argv[0] */
- retval = copy_strings_kernel(1, &fmt->interpreter, bprm);
+ retval = copy_string_kernel(fmt->interpreter, bprm);
if (retval < 0)
goto error;
bprm->argc++;
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index e9e6a6f4a35f..c4fb7f52a46e 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -117,17 +117,17 @@ static int load_script(struct linux_binprm *bprm)
retval = remove_arg_zero(bprm);
if (retval)
return retval;
- retval = copy_strings_kernel(1, &bprm->interp, bprm);
+ retval = copy_string_kernel(bprm->interp, bprm);
if (retval < 0)
return retval;
bprm->argc++;
if (i_arg) {
- retval = copy_strings_kernel(1, &i_arg, bprm);
+ retval = copy_string_kernel(i_arg, bprm);
if (retval < 0)
return retval;
bprm->argc++;
}
- retval = copy_strings_kernel(1, &i_name, bprm);
+ retval = copy_string_kernel(i_name, bprm);
if (retval)
return retval;
bprm->argc++;
diff --git a/fs/exec.c b/fs/exec.c
index 06b4c550af5d..b2a77d5acede 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -588,24 +588,23 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
}
/*
- * Like copy_strings, but get argv and its values from kernel memory.
+ * Copy and argument/environment string from the kernel to the processes stack.
*/
-int copy_strings_kernel(int argc, const char *const *__argv,
- struct linux_binprm *bprm)
+int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
{
int r;
mm_segment_t oldfs = get_fs();
struct user_arg_ptr argv = {
- .ptr.native = (const char __user *const __user *)__argv,
+ .ptr.native = (const char __user *const __user *)&arg,
};
set_fs(KERNEL_DS);
- r = copy_strings(argc, argv, bprm);
+ r = copy_strings(1, argv, bprm);
set_fs(oldfs);
return r;
}
-EXPORT_SYMBOL(copy_strings_kernel);
+EXPORT_SYMBOL(copy_string_kernel);
#ifdef CONFIG_MMU
@@ -1863,7 +1862,7 @@ static int __do_execve_file(int fd, struct filename *filename,
if (retval < 0)
goto out;
- retval = copy_strings_kernel(1, &bprm->filename, bprm);
+ retval = copy_string_kernel(bprm->filename, bprm);
if (retval < 0)
goto out;
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index a345d9fed3d8..3d3afe094c97 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -144,8 +144,7 @@ extern int setup_arg_pages(struct linux_binprm * bprm,
extern int transfer_args_to_stack(struct linux_binprm *bprm,
unsigned long *sp_location);
extern int bprm_change_interp(const char *interp, struct linux_binprm *bprm);
-extern int copy_strings_kernel(int argc, const char *const *argv,
- struct linux_binprm *bprm);
+int copy_string_kernel(const char *arg, struct linux_binprm *bprm);
extern void install_exec_creds(struct linux_binprm *bprm);
extern void set_binfmt(struct linux_binfmt *new);
extern ssize_t read_code(struct file *, unsigned long, loff_t, size_t);
--
2.25.1
^ permalink raw reply related
* [PATCH 6/6] exec: open code copy_string_kernel
From: Christoph Hellwig @ 2020-04-06 12:03 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: linux-fsdevel, linuxppc-dev, Jeremy Kerr, Arnd Bergmann,
linux-kernel
In-Reply-To: <20200406120312.1150405-1-hch@lst.de>
Currently copy_string_kernel is just a wrapper around copy_strings that
simplifies the calling conventions and uses set_fs to allow passing a
kernel pointer. But due to the fact the we only need to handle a single
kernel argument pointer, the logic can be sigificantly simplified while
getting rid of the set_fs.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/exec.c | 43 ++++++++++++++++++++++++++++++++++---------
1 file changed, 34 insertions(+), 9 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index b2a77d5acede..7c64cae8ad0a 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -592,17 +592,42 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
*/
int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
{
- int r;
- mm_segment_t oldfs = get_fs();
- struct user_arg_ptr argv = {
- .ptr.native = (const char __user *const __user *)&arg,
- };
+ int len = strnlen(arg, MAX_ARG_STRLEN) + 1 /* terminating null */;
+ unsigned long pos = bprm->p;
+
+ if (len == 0)
+ return -EFAULT;
+ if (!valid_arg_len(bprm, len))
+ return -E2BIG;
+
+ /* We're going to work our way backwords. */
+ arg += len;
+ bprm->p -= len;
+ if (IS_ENABLED(CONFIG_MMU) && bprm->p < bprm->argmin)
+ return -E2BIG;
+
+ while (len > 0) {
+ unsigned int bytes_to_copy = min_t(unsigned int, len,
+ min_not_zero(offset_in_page(pos), PAGE_SIZE));
+ struct page *page;
+ char *kaddr;
- set_fs(KERNEL_DS);
- r = copy_strings(1, argv, bprm);
- set_fs(oldfs);
+ pos -= bytes_to_copy;
+ arg -= bytes_to_copy;
+ len -= bytes_to_copy;
- return r;
+ page = get_arg_page(bprm, pos, 1);
+ if (!page)
+ return -E2BIG;
+ kaddr = kmap_atomic(page);
+ flush_arg_page(bprm, pos & PAGE_MASK, page);
+ memcpy(kaddr + offset_in_page(pos), arg, bytes_to_copy);
+ flush_kernel_dcache_page(page);
+ kunmap_atomic(kaddr);
+ put_arg_page(page);
+ }
+
+ return 0;
}
EXPORT_SYMBOL(copy_string_kernel);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 6/6] exec: open code copy_string_kernel
From: Matthew Wilcox @ 2020-04-06 12:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Arnd Bergmann, linux-kernel, Alexander Viro, linux-fsdevel,
Andrew Morton, linuxppc-dev, Jeremy Kerr
In-Reply-To: <20200406120312.1150405-7-hch@lst.de>
On Mon, Apr 06, 2020 at 02:03:12PM +0200, Christoph Hellwig wrote:
> + int len = strnlen(arg, MAX_ARG_STRLEN) + 1 /* terminating null */;
If you end up doing another version of this, it's a terminating NUL, not null.
I almost wonder if we shouldn't have
#define TERMINATING_NUL 1
in kernel.h.
int len = strnlen(arg, MAX_ARG_STRLEN) + TERMINATING_NUL;
has a certain appeal. There's the risk people might misuse it though ...
str[end] = TERMINATING_NUL;
so probably not a good idea.
^ permalink raw reply
* [Bug 207129] PowerMac G4 DP (5.6.2 debug kernel + inline KASAN) freezes shortly after booting with "do_IRQ: stack overflow: 1760"
From: bugzilla-daemon @ 2020-04-06 12:26 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-207129-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=207129
--- Comment #2 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 288229
--> https://bugzilla.kernel.org/attachment.cgi?id=288229&action=edit
screenshot01.jpg
Without CONFIG_DEBUG_STACKOVERFLOW things are better. The rsync completes, the
G4 was building stuff for 2 hours or so until I got these errors and a hard
freeze:
[...]
Oops: kernel stack overflow, sig: 11 [#1]
BE PAGE_SIZE=4K MMU=Hash SMP NR_CPUS=2 PowerMac
Modules linked in: ...
CPU: 1 PID: 17105 Comm: kworker/u4:5 Tainted: G W
5.6.2-PowerMacG4+ #5
------------[ cut here ]------------
kernel BUG at mm/usercopy.c:99!
Oops: Exception in kernel mode, sig: 5 [#2]
BE PAGE_SIZE=4K MMU=Hash SMP NR_CPUS=2 PowerMac
Modules linked in: ...
CPU: 1 PID: 17185 Comm: kworker/u4:5 Tainted: G W
5.6.2-PowerMacG4+ #5
usercopy: Kernel memory overwrite attempt detected to kernel text (offset 6336,
size 4)!
------------[ cut here ]------------
kernel BUG at mm/usercopy.c:99!
Oops: Exception in kernel mode, sig: 5 [#3]
BE PAGE_SIZE=4K MMU=Hash SMP NR_CPUS=2 PowerMac
Modules linked in: ...
CPU: 1 PID: 17185 Comm: kworker/u4:5 Tainted: G W
5.6.2-PowerMacG4+ #5
usercopy: Kernel memory overwrite attempt detected to kernel text (offset 5336,
size 4)!
------------[ cut here ]------------
kernel BUG at mm/usercopy.c:99!
Oops: Exception in kernel mode, sig: 5 [#4]
BE PAGE_SIZE=4K MMU=Hash SMP NR_CPUS=2 PowerMac
Modules linked in: ...
CPU: 1 PID: 17185 Comm: kworker/u4:5 Tainted: G W
5.6.2-PowerMacG4+ #5
usercopy: Kernel memory overwrite attempt detected to kernel text (offset 4336,
size 4)!
------------[ cut here ]------------
kernel BUG at mm/usercopy.c:99!
Oops: Exception in kernel mode, sig: 5 [#5]
BE PAGE_SIZE=4K MMU=Hash SMP NR_CPUS=2 PowerMac
Modules linked in: ...
Unrecoverable FP Unavailable Exception 801 at 9b8
CPU: 1 PID: 17185 Comm: kworker/u4:5 Tainted: G W
5.6.2-PowerMacG4+ #5
usercopy: Kernel memory overwrite attempt detected to kernel text (offset 3336,
size 4)!
------------[ cut here ]------------
Now running with CONFIG_THREAD_SHIFT=14 which runs fine so far... Did not try
without CONFIG_VMAP_STACK yet.
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 207129] PowerMac G4 DP (5.6.2 debug kernel + inline KASAN) freezes shortly after booting with "do_IRQ: stack overflow: 1760"
From: bugzilla-daemon @ 2020-04-06 12:27 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-207129-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=207129
--- Comment #3 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 288231
--> https://bugzilla.kernel.org/attachment.cgi?id=288231&action=edit
screenshot02.jpg
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [PATCH v5 1/2] powerpc/powernv: Remove redundant assignments to attr and name
From: Qiujun Huang @ 2020-04-06 12:52 UTC (permalink / raw)
To: benh, paulus, mpe, oohall
Cc: linux-kernel, Markus.Elfring, tglx, linuxppc-dev, Qiujun Huang
In-Reply-To: <20200406125242.4973-1-hqjagain@gmail.com>
We don't need to go to the labal of out when of_property_read_u64_array
fails, as there is nothing to do. Just return.
And we can remove the redundant assignments to attr and name.
Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
---
arch/powerpc/platforms/powernv/opal.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 2b3dfd0b6cdd..71af1cbc6334 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -801,14 +801,14 @@ static ssize_t export_attr_read(struct file *fp, struct kobject *kobj,
static int opal_add_one_export(struct kobject *parent, const char *export_name,
struct device_node *np, const char *prop_name)
{
- struct bin_attribute *attr = NULL;
- const char *name = NULL;
+ struct bin_attribute *attr;
+ const char *name;
u64 vals[2];
int rc;
rc = of_property_read_u64_array(np, prop_name, &vals[0], 2);
if (rc)
- goto out;
+ return rc;
attr = kzalloc(sizeof(*attr), GFP_KERNEL);
name = kstrdup(export_name, GFP_KERNEL);
--
2.17.1
^ permalink raw reply related
* [PATCH v5 0/2] Fix opal_add_one_export
From: Qiujun Huang @ 2020-04-06 12:52 UTC (permalink / raw)
To: benh, paulus, mpe, oohall
Cc: linux-kernel, Markus.Elfring, tglx, linuxppc-dev, Qiujun Huang
We should check the return value of kzalloc, as kzalloc may fail returning NULL.
---
v4->v5:
Separate the patch into two.
v3->v4:
Added the information about coccinelle script.
Added change log.
Added Oliver's Reviewed-by.
v2->v3:
Removed redundant assignment to 'attr' and 'name'.
v1->v2:
Just return -ENOMEM if attr is NULL.
Qiujun Huang (2):
powerpc/powernv: Remove redundant assignments to attr and name
powerpc/powernv: Add NULL check after kzalloc in opal_add_one_export
arch/powerpc/platforms/powernv/opal.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v5 2/2] powerpc/powernv: Add NULL check after kzalloc in opal_add_one_export
From: Qiujun Huang @ 2020-04-06 12:52 UTC (permalink / raw)
To: benh, paulus, mpe, oohall
Cc: linux-kernel, Markus.Elfring, tglx, linuxppc-dev, Qiujun Huang
In-Reply-To: <20200406125242.4973-1-hqjagain@gmail.com>
Here needs a NULL check, as kzalloc may fail returning NULL.
Issue was found by coccinelle.
Generated by: scripts/coccinelle/null/kmerr.cocci
Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
---
arch/powerpc/platforms/powernv/opal.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 71af1cbc6334..908d749bcef5 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -811,6 +811,9 @@ static int opal_add_one_export(struct kobject *parent, const char *export_name,
return rc;
attr = kzalloc(sizeof(*attr), GFP_KERNEL);
+ if (!attr)
+ return -ENOMEM;
+
name = kstrdup(export_name, GFP_KERNEL);
if (!name) {
rc = -ENOMEM;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 2/6] binfmt_elf: open code copy_siginfo_to_user to kernelspace buffer
From: Arnd Bergmann @ 2020-04-06 13:01 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-kernel@vger.kernel.org, Alexander Viro,
Linux FS-devel Mailing List, Andrew Morton, linuxppc-dev,
Jeremy Kerr
In-Reply-To: <20200406120312.1150405-3-hch@lst.de>
On Mon, Apr 6, 2020 at 2:03 PM Christoph Hellwig <hch@lst.de> wrote:
>
> Instead of messing with the address limit just open code the trivial
> memcpy + memset logic.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/binfmt_elf.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index f4713ea76e82..d744ce9a4b52 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1556,10 +1556,9 @@ static void fill_auxv_note(struct memelfnote *note, struct mm_struct *mm)
> static void fill_siginfo_note(struct memelfnote *note, user_siginfo_t *csigdata,
> const kernel_siginfo_t *siginfo)
> {
> - mm_segment_t old_fs = get_fs();
> - set_fs(KERNEL_DS);
> - copy_siginfo_to_user((user_siginfo_t __user *) csigdata, siginfo);
> - set_fs(old_fs);
> + memcpy(csigdata, siginfo, sizeof(struct kernel_siginfo));
> + memset((char *)csigdata + sizeof(struct kernel_siginfo), 0,
> + SI_EXPANSION_SIZE);
> fill_note(note, "CORE", NT_SIGINFO, sizeof(*csigdata), csigdata);
> }
I think this breaks compat binfmt-elf mode, which relies on this trick:
fs/compat_binfmt_elf.c:#define copy_siginfo_to_user copy_siginfo_to_user32
fs/compat_binfmt_elf.c#include "binfmt_elf.c"
At least we seem to only have one remaining implementation of
__copy_siginfo_to_user32(), so fixing this won't require touching all
architectures, but I don't see an obvious way to do it right. Maybe
compat-binfmt-elf.c should just override fill_siginfo_note() itself
rather than overriding copy_siginfo_to_user().
Arnd
^ permalink raw reply
* Re: [PATCH 3/6] binfmt_elf: remove the set_fs(KERNEL_DS) in elf_core_dump
From: Al Viro @ 2020-04-06 13:02 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
Andrew Morton, linuxppc-dev
In-Reply-To: <20200406120312.1150405-4-hch@lst.de>
On Mon, Apr 06, 2020 at 02:03:09PM +0200, Christoph Hellwig wrote:
> There is no logic in elf_core_dump itself that uses uaccess routines
> on kernel pointers, the file writes are nicely encapsulated in dump_emit
> which does its own set_fs.
... assuming you've checked the asm/elf.h to see that nobody is playing
silly buggers in these forests of macros and the stuff called from those.
Which is a feat that ought to be mentioned in commit message...
^ permalink raw reply
* Re: [PATCH 3/6] binfmt_elf: remove the set_fs(KERNEL_DS) in elf_core_dump
From: Christoph Hellwig @ 2020-04-06 13:03 UTC (permalink / raw)
To: Al Viro
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
Andrew Morton, linuxppc-dev, Christoph Hellwig
In-Reply-To: <20200406130238.GT23230@ZenIV.linux.org.uk>
On Mon, Apr 06, 2020 at 02:02:38PM +0100, Al Viro wrote:
> On Mon, Apr 06, 2020 at 02:03:09PM +0200, Christoph Hellwig wrote:
> > There is no logic in elf_core_dump itself that uses uaccess routines
> > on kernel pointers, the file writes are nicely encapsulated in dump_emit
> > which does its own set_fs.
>
> ... assuming you've checked the asm/elf.h to see that nobody is playing
> silly buggers in these forests of macros and the stuff called from those.
> Which is a feat that ought to be mentioned in commit message...
None of the calls should go into asm/elf.h headers, but some go to
various out of line arch callouts. And I did look through those - spufs
was the only funky one.
^ 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