| Message ID | 20221010095346.1957-2-thunder.leizhen@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: Make the dumped instructions are consistent with the disassembled ones | expand |
On Mon, 10 Oct 2022 at 11:56, Zhen Lei <thunder.leizhen@huawei.com> wrote: > > Fix the following warnings: > warning: incorrect type in initializer (different address spaces) > expected unsigned short [noderef] __user *register __p > got unsigned short [usertype] * > warning: cast removes address space '__user' of expression > > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > --- > arch/arm/kernel/traps.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > index 20b2db6dcd1ced7..34aa80c09c508c1 100644 > --- a/arch/arm/kernel/traps.c > +++ b/arch/arm/kernel/traps.c > @@ -188,9 +188,9 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) > } > } else { > if (thumb) > - bad = get_user(val, &((u16 *)addr)[i]); > + bad = get_user(val, &((u16 __user *)addr)[i]); > else > - bad = get_user(val, &((u32 *)addr)[i]); > + bad = get_user(val, &((u32 __user *)addr)[i]); > } > > if (!bad) > @@ -455,15 +455,15 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) > if (processor_mode(regs) == SVC_MODE) { > #ifdef CONFIG_THUMB2_KERNEL > if (thumb_mode(regs)) { > - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > + instr = __mem_to_opcode_thumb16(((__force u16 *)pc)[0]); Shouldn't this be __user as well? (and below) > if (is_wide_instruction(instr)) { > u16 inst2; > - inst2 = __mem_to_opcode_thumb16(((u16 *)pc)[1]); > + inst2 = __mem_to_opcode_thumb16(((__force u16 *)pc)[1]); > instr = __opcode_thumb32_compose(instr, inst2); > } > } else > #endif > - instr = __mem_to_opcode_arm(*(u32 *) pc); > + instr = __mem_to_opcode_arm(*(__force u32 *) pc); > } else if (thumb_mode(regs)) { > if (get_user(instr, (u16 __user *)pc)) > goto die_sig; > -- > 2.25.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 2022/10/10 18:20, Ard Biesheuvel wrote: > On Mon, 10 Oct 2022 at 11:56, Zhen Lei <thunder.leizhen@huawei.com> wrote: >> >> Fix the following warnings: >> warning: incorrect type in initializer (different address spaces) >> expected unsigned short [noderef] __user *register __p >> got unsigned short [usertype] * >> warning: cast removes address space '__user' of expression >> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >> --- >> arch/arm/kernel/traps.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c >> index 20b2db6dcd1ced7..34aa80c09c508c1 100644 >> --- a/arch/arm/kernel/traps.c >> +++ b/arch/arm/kernel/traps.c >> @@ -188,9 +188,9 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) >> } >> } else { >> if (thumb) >> - bad = get_user(val, &((u16 *)addr)[i]); >> + bad = get_user(val, &((u16 __user *)addr)[i]); >> else >> - bad = get_user(val, &((u32 *)addr)[i]); >> + bad = get_user(val, &((u32 __user *)addr)[i]); >> } >> >> if (!bad) >> @@ -455,15 +455,15 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) >> if (processor_mode(regs) == SVC_MODE) { >> #ifdef CONFIG_THUMB2_KERNEL >> if (thumb_mode(regs)) { >> - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); >> + instr = __mem_to_opcode_thumb16(((__force u16 *)pc)[0]); > > Shouldn't this be __user as well? (and below) unsigned int instr; void __user *pc; The __user can clear the warning, but a new warning will be generated. instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); ^new ^old arch/arm/kernel/traps.c:473:33: warning: dereference of noderef expression > >> if (is_wide_instruction(instr)) { >> u16 inst2; >> - inst2 = __mem_to_opcode_thumb16(((u16 *)pc)[1]); >> + inst2 = __mem_to_opcode_thumb16(((__force u16 *)pc)[1]); >> instr = __opcode_thumb32_compose(instr, inst2); >> } >> } else >> #endif >> - instr = __mem_to_opcode_arm(*(u32 *) pc); >> + instr = __mem_to_opcode_arm(*(__force u32 *) pc); >> } else if (thumb_mode(regs)) { >> if (get_user(instr, (u16 __user *)pc)) >> goto die_sig; >> -- >> 2.25.1 >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > . >
On Mon, 10 Oct 2022 at 12:58, Leizhen (ThunderTown) <thunder.leizhen@huawei.com> wrote: > > > > On 2022/10/10 18:20, Ard Biesheuvel wrote: > > On Mon, 10 Oct 2022 at 11:56, Zhen Lei <thunder.leizhen@huawei.com> wrote: > >> > >> Fix the following warnings: > >> warning: incorrect type in initializer (different address spaces) > >> expected unsigned short [noderef] __user *register __p > >> got unsigned short [usertype] * > >> warning: cast removes address space '__user' of expression > >> > >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > >> --- > >> arch/arm/kernel/traps.c | 10 +++++----- > >> 1 file changed, 5 insertions(+), 5 deletions(-) > >> > >> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > >> index 20b2db6dcd1ced7..34aa80c09c508c1 100644 > >> --- a/arch/arm/kernel/traps.c > >> +++ b/arch/arm/kernel/traps.c > >> @@ -188,9 +188,9 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) > >> } > >> } else { > >> if (thumb) > >> - bad = get_user(val, &((u16 *)addr)[i]); > >> + bad = get_user(val, &((u16 __user *)addr)[i]); > >> else > >> - bad = get_user(val, &((u32 *)addr)[i]); > >> + bad = get_user(val, &((u32 __user *)addr)[i]); > >> } > >> > >> if (!bad) > >> @@ -455,15 +455,15 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) > >> if (processor_mode(regs) == SVC_MODE) { > >> #ifdef CONFIG_THUMB2_KERNEL > >> if (thumb_mode(regs)) { > >> - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > >> + instr = __mem_to_opcode_thumb16(((__force u16 *)pc)[0]); > > > > Shouldn't this be __user as well? (and below) > > unsigned int instr; > void __user *pc; > > The __user can clear the warning, but a new warning will be generated. > > instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > ^new ^old > > arch/arm/kernel/traps.c:473:33: warning: dereference of noderef expression > This is because dereferencing a __user pointer is not permitted. So this code should be using get_kernel_nofault() here not a plain dereference of PC. So better to fix that properly instead of papering over it with a __force cast just to make sparse happy. > > > >> if (is_wide_instruction(instr)) { > >> u16 inst2; > >> - inst2 = __mem_to_opcode_thumb16(((u16 *)pc)[1]); > >> + inst2 = __mem_to_opcode_thumb16(((__force u16 *)pc)[1]); > >> instr = __opcode_thumb32_compose(instr, inst2); > >> } > >> } else > >> #endif > >> - instr = __mem_to_opcode_arm(*(u32 *) pc); > >> + instr = __mem_to_opcode_arm(*(__force u32 *) pc); > >> } else if (thumb_mode(regs)) { > >> if (get_user(instr, (u16 __user *)pc)) > >> goto die_sig; > >> -- > >> 2.25.1 > >> > >> > >> _______________________________________________ > >> linux-arm-kernel mailing list > >> linux-arm-kernel@lists.infradead.org > >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > . > > > > -- > Regards, > Zhen Lei
On Mon, Oct 10, 2022 at 05:53:45PM +0800, Zhen Lei wrote: > Fix the following warnings: > warning: incorrect type in initializer (different address spaces) > expected unsigned short [noderef] __user *register __p > got unsigned short [usertype] * > warning: cast removes address space '__user' of expression I have a general principle that not all warnings should be fixed, especially when it comes to sparse. The aim is not to get to zero warnings - it's to get to a point where the code is correct, and plastering the code with __force casts means it isn't correct - you're just masking the warning. So no, I really don't like this. And I really don't like seeing __force being used in open code in casts.
On Mon, Oct 10, 2022 at 01:06:19PM +0200, Ard Biesheuvel wrote: > On Mon, 10 Oct 2022 at 12:58, Leizhen (ThunderTown) > <thunder.leizhen@huawei.com> wrote: > > > > > > > > On 2022/10/10 18:20, Ard Biesheuvel wrote: > > > On Mon, 10 Oct 2022 at 11:56, Zhen Lei <thunder.leizhen@huawei.com> wrote: > > >> > > >> Fix the following warnings: > > >> warning: incorrect type in initializer (different address spaces) > > >> expected unsigned short [noderef] __user *register __p > > >> got unsigned short [usertype] * > > >> warning: cast removes address space '__user' of expression > > >> > > >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > > >> --- > > >> arch/arm/kernel/traps.c | 10 +++++----- > > >> 1 file changed, 5 insertions(+), 5 deletions(-) > > >> > > >> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > > >> index 20b2db6dcd1ced7..34aa80c09c508c1 100644 > > >> --- a/arch/arm/kernel/traps.c > > >> +++ b/arch/arm/kernel/traps.c > > >> @@ -188,9 +188,9 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) > > >> } > > >> } else { > > >> if (thumb) > > >> - bad = get_user(val, &((u16 *)addr)[i]); > > >> + bad = get_user(val, &((u16 __user *)addr)[i]); > > >> else > > >> - bad = get_user(val, &((u32 *)addr)[i]); > > >> + bad = get_user(val, &((u32 __user *)addr)[i]); > > >> } > > >> > > >> if (!bad) > > >> @@ -455,15 +455,15 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) > > >> if (processor_mode(regs) == SVC_MODE) { > > >> #ifdef CONFIG_THUMB2_KERNEL > > >> if (thumb_mode(regs)) { > > >> - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > > >> + instr = __mem_to_opcode_thumb16(((__force u16 *)pc)[0]); > > > > > > Shouldn't this be __user as well? (and below) > > > > unsigned int instr; > > void __user *pc; > > > > The __user can clear the warning, but a new warning will be generated. > > > > instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > > ^new ^old > > > > arch/arm/kernel/traps.c:473:33: warning: dereference of noderef expression > > > > This is because dereferencing a __user pointer is not permitted. > > So this code should be using get_kernel_nofault() here not a plain > dereference of PC. So better to fix that properly instead of papering > over it with a __force cast just to make sparse happy. Why? We won't get here unless the PC can be dereferenced. If it's not able to be dereferenced, then we'd be dealing with a prefetch abort.
On Mon, 10 Oct 2022 at 18:08, Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Mon, Oct 10, 2022 at 01:06:19PM +0200, Ard Biesheuvel wrote: > > On Mon, 10 Oct 2022 at 12:58, Leizhen (ThunderTown) > > <thunder.leizhen@huawei.com> wrote: > > > > > > > > > > > > On 2022/10/10 18:20, Ard Biesheuvel wrote: > > > > On Mon, 10 Oct 2022 at 11:56, Zhen Lei <thunder.leizhen@huawei.com> wrote: > > > >> > > > >> Fix the following warnings: > > > >> warning: incorrect type in initializer (different address spaces) > > > >> expected unsigned short [noderef] __user *register __p > > > >> got unsigned short [usertype] * > > > >> warning: cast removes address space '__user' of expression > > > >> > > > >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > > > >> --- > > > >> arch/arm/kernel/traps.c | 10 +++++----- > > > >> 1 file changed, 5 insertions(+), 5 deletions(-) > > > >> > > > >> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > > > >> index 20b2db6dcd1ced7..34aa80c09c508c1 100644 > > > >> --- a/arch/arm/kernel/traps.c > > > >> +++ b/arch/arm/kernel/traps.c > > > >> @@ -188,9 +188,9 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) > > > >> } > > > >> } else { > > > >> if (thumb) > > > >> - bad = get_user(val, &((u16 *)addr)[i]); > > > >> + bad = get_user(val, &((u16 __user *)addr)[i]); > > > >> else > > > >> - bad = get_user(val, &((u32 *)addr)[i]); > > > >> + bad = get_user(val, &((u32 __user *)addr)[i]); > > > >> } > > > >> > > > >> if (!bad) > > > >> @@ -455,15 +455,15 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) > > > >> if (processor_mode(regs) == SVC_MODE) { > > > >> #ifdef CONFIG_THUMB2_KERNEL > > > >> if (thumb_mode(regs)) { > > > >> - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > > > >> + instr = __mem_to_opcode_thumb16(((__force u16 *)pc)[0]); > > > > > > > > Shouldn't this be __user as well? (and below) > > > > > > unsigned int instr; > > > void __user *pc; > > > > > > The __user can clear the warning, but a new warning will be generated. > > > > > > instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > > > ^new ^old > > > > > > arch/arm/kernel/traps.c:473:33: warning: dereference of noderef expression > > > > > > > This is because dereferencing a __user pointer is not permitted. > > > > So this code should be using get_kernel_nofault() here not a plain > > dereference of PC. So better to fix that properly instead of papering > > over it with a __force cast just to make sparse happy. > > Why? We won't get here unless the PC can be dereferenced. If it's not > able to be dereferenced, then we'd be dealing with a prefetch abort. > If that is guaranteed (i.e., there is no way we might be racing with a module unload on another CPU or something like that), then I agree that dereferencing PC is fine.
On Mon, Oct 10, 2022 at 06:14:56PM +0200, Ard Biesheuvel wrote: > On Mon, 10 Oct 2022 at 18:08, Russell King (Oracle) > <linux@armlinux.org.uk> wrote: > > > > On Mon, Oct 10, 2022 at 01:06:19PM +0200, Ard Biesheuvel wrote: > > > On Mon, 10 Oct 2022 at 12:58, Leizhen (ThunderTown) > > > <thunder.leizhen@huawei.com> wrote: > > > > > > > > > > > > > > > > On 2022/10/10 18:20, Ard Biesheuvel wrote: > > > > > On Mon, 10 Oct 2022 at 11:56, Zhen Lei <thunder.leizhen@huawei.com> wrote: > > > > >> > > > > >> Fix the following warnings: > > > > >> warning: incorrect type in initializer (different address spaces) > > > > >> expected unsigned short [noderef] __user *register __p > > > > >> got unsigned short [usertype] * > > > > >> warning: cast removes address space '__user' of expression > > > > >> > > > > >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > > > > >> --- > > > > >> arch/arm/kernel/traps.c | 10 +++++----- > > > > >> 1 file changed, 5 insertions(+), 5 deletions(-) > > > > >> > > > > >> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > > > > >> index 20b2db6dcd1ced7..34aa80c09c508c1 100644 > > > > >> --- a/arch/arm/kernel/traps.c > > > > >> +++ b/arch/arm/kernel/traps.c > > > > >> @@ -188,9 +188,9 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) > > > > >> } > > > > >> } else { > > > > >> if (thumb) > > > > >> - bad = get_user(val, &((u16 *)addr)[i]); > > > > >> + bad = get_user(val, &((u16 __user *)addr)[i]); > > > > >> else > > > > >> - bad = get_user(val, &((u32 *)addr)[i]); > > > > >> + bad = get_user(val, &((u32 __user *)addr)[i]); > > > > >> } > > > > >> > > > > >> if (!bad) > > > > >> @@ -455,15 +455,15 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) > > > > >> if (processor_mode(regs) == SVC_MODE) { > > > > >> #ifdef CONFIG_THUMB2_KERNEL > > > > >> if (thumb_mode(regs)) { > > > > >> - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > > > > >> + instr = __mem_to_opcode_thumb16(((__force u16 *)pc)[0]); > > > > > > > > > > Shouldn't this be __user as well? (and below) > > > > > > > > unsigned int instr; > > > > void __user *pc; > > > > > > > > The __user can clear the warning, but a new warning will be generated. > > > > > > > > instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > > > > ^new ^old > > > > > > > > arch/arm/kernel/traps.c:473:33: warning: dereference of noderef expression > > > > > > > > > > This is because dereferencing a __user pointer is not permitted. > > > > > > So this code should be using get_kernel_nofault() here not a plain > > > dereference of PC. So better to fix that properly instead of papering > > > over it with a __force cast just to make sparse happy. > > > > Why? We won't get here unless the PC can be dereferenced. If it's not > > able to be dereferenced, then we'd be dealing with a prefetch abort. > > > > If that is guaranteed (i.e., there is no way we might be racing with a > module unload on another CPU or something like that), then I agree > that dereferencing PC is fine. If we get here for an instruction in a module that's being unloaded, we have way bigger problems. We shouldn't be executing code in a module being unloaded in the first place. That becomes a case of "deserves to oops". The more likely case would be a prefetch abort when the page is unmapped. You'd have to try pretty hard to get an undef to race with a module unload.
On 2022/10/11 0:05, Russell King (Oracle) wrote: > On Mon, Oct 10, 2022 at 05:53:45PM +0800, Zhen Lei wrote: >> Fix the following warnings: >> warning: incorrect type in initializer (different address spaces) >> expected unsigned short [noderef] __user *register __p >> got unsigned short [usertype] * >> warning: cast removes address space '__user' of expression > > I have a general principle that not all warnings should be fixed, > especially when it comes to sparse. OK, I got it. > > The aim is not to get to zero warnings - it's to get to a point where > the code is correct, and plastering the code with __force casts means > it isn't correct - you're just masking the warning. > > So no, I really don't like this. And I really don't like seeing > __force being used in open code in casts. OK, I will clear only the first warning and leave the second warning. >
On 2022/10/10 19:06, Ard Biesheuvel wrote: > On Mon, 10 Oct 2022 at 12:58, Leizhen (ThunderTown) > <thunder.leizhen@huawei.com> wrote: >> >> >> >> On 2022/10/10 18:20, Ard Biesheuvel wrote: >>> On Mon, 10 Oct 2022 at 11:56, Zhen Lei <thunder.leizhen@huawei.com> wrote: >>>> >>>> Fix the following warnings: >>>> warning: incorrect type in initializer (different address spaces) >>>> expected unsigned short [noderef] __user *register __p >>>> got unsigned short [usertype] * >>>> warning: cast removes address space '__user' of expression >>>> >>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >>>> --- >>>> arch/arm/kernel/traps.c | 10 +++++----- >>>> 1 file changed, 5 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c >>>> index 20b2db6dcd1ced7..34aa80c09c508c1 100644 >>>> --- a/arch/arm/kernel/traps.c >>>> +++ b/arch/arm/kernel/traps.c >>>> @@ -188,9 +188,9 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) >>>> } >>>> } else { >>>> if (thumb) >>>> - bad = get_user(val, &((u16 *)addr)[i]); >>>> + bad = get_user(val, &((u16 __user *)addr)[i]); >>>> else >>>> - bad = get_user(val, &((u32 *)addr)[i]); >>>> + bad = get_user(val, &((u32 __user *)addr)[i]); >>>> } >>>> >>>> if (!bad) >>>> @@ -455,15 +455,15 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) >>>> if (processor_mode(regs) == SVC_MODE) { >>>> #ifdef CONFIG_THUMB2_KERNEL >>>> if (thumb_mode(regs)) { >>>> - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); >>>> + instr = __mem_to_opcode_thumb16(((__force u16 *)pc)[0]); >>> >>> Shouldn't this be __user as well? (and below) >> >> unsigned int instr; >> void __user *pc; >> >> The __user can clear the warning, but a new warning will be generated. >> >> instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); >> ^new ^old >> >> arch/arm/kernel/traps.c:473:33: warning: dereference of noderef expression >> > > This is because dereferencing a __user pointer is not permitted. > > So this code should be using get_kernel_nofault() here not a plain > dereference of PC. So better to fix that properly instead of papering > over it with a __force cast just to make sparse happy. How about: @@ -451,9 +451,9 @@ int call_undef_hook(struct pt_regs *regs, unsigned int instr) asmlinkage void do_undefinstr(struct pt_regs *regs) { unsigned int instr; - void __user *pc; + void *pc; - pc = (void __user *)instruction_pointer(regs); + pc = (void *)instruction_pointer(regs); if (processor_mode(regs) == SVC_MODE) { #ifdef CONFIG_THUMB2_KERNEL @@ -497,7 +497,7 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) } #endif arm_notify_die("Oops - undefined instruction", regs, - SIGILL, ILL_ILLOPC, pc, 0, 6); + SIGILL, ILL_ILLOPC, (void __user *)pc, 0, 6); } NOKPROBE_SYMBOL(do_undefinstr) The 'pc' may come from kernel or user. And I found that all the get_user() calls have already done type casts to the pc, except arm_notify_die(). I think the above changes are reasonable. > > >>> >>>> if (is_wide_instruction(instr)) { >>>> u16 inst2; >>>> - inst2 = __mem_to_opcode_thumb16(((u16 *)pc)[1]); >>>> + inst2 = __mem_to_opcode_thumb16(((__force u16 *)pc)[1]); >>>> instr = __opcode_thumb32_compose(instr, inst2); >>>> } >>>> } else >>>> #endif >>>> - instr = __mem_to_opcode_arm(*(u32 *) pc); >>>> + instr = __mem_to_opcode_arm(*(__force u32 *) pc); >>>> } else if (thumb_mode(regs)) { >>>> if (get_user(instr, (u16 __user *)pc)) >>>> goto die_sig; >>>> -- >>>> 2.25.1 >>>> >>>> >>>> _______________________________________________ >>>> linux-arm-kernel mailing list >>>> linux-arm-kernel@lists.infradead.org >>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >>> . >>> >> >> -- >> Regards, >> Zhen Lei > . >
On 2022/10/11 10:13, Leizhen (ThunderTown) wrote: > > > On 2022/10/11 0:05, Russell King (Oracle) wrote: >> On Mon, Oct 10, 2022 at 05:53:45PM +0800, Zhen Lei wrote: >>> Fix the following warnings: >>> warning: incorrect type in initializer (different address spaces) >>> expected unsigned short [noderef] __user *register __p >>> got unsigned short [usertype] * >>> warning: cast removes address space '__user' of expression >> >> I have a general principle that not all warnings should be fixed, >> especially when it comes to sparse. > > OK, I got it. > >> >> The aim is not to get to zero warnings - it's to get to a point where >> the code is correct, and plastering the code with __force casts means >> it isn't correct - you're just masking the warning. >> >> So no, I really don't like this. And I really don't like seeing >> __force being used in open code in casts. > > OK, I will clear only the first warning and leave the second warning. Sorry, Maybe I misunderstood. '__force casts' mentioned here may include not only '(__force u16 *)', but also '(u16 __user *)'. > >> >
On Tue, Oct 11, 2022 at 10:29:58AM +0800, Leizhen (ThunderTown) wrote: > On 2022/10/10 19:06, Ard Biesheuvel wrote: > > On Mon, 10 Oct 2022 at 12:58, Leizhen (ThunderTown) > > <thunder.leizhen@huawei.com> wrote: > >> On 2022/10/10 18:20, Ard Biesheuvel wrote: > >>> On Mon, 10 Oct 2022 at 11:56, Zhen Lei <thunder.leizhen@huawei.com> wrote: > >>>> > >>>> Fix the following warnings: > >>>> warning: incorrect type in initializer (different address spaces) > >>>> expected unsigned short [noderef] __user *register __p > >>>> got unsigned short [usertype] * > >>>> warning: cast removes address space '__user' of expression > >>>> > >>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > >>>> --- > >>>> arch/arm/kernel/traps.c | 10 +++++----- > >>>> 1 file changed, 5 insertions(+), 5 deletions(-) > >>>> > >>>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > >>>> index 20b2db6dcd1ced7..34aa80c09c508c1 100644 > >>>> --- a/arch/arm/kernel/traps.c > >>>> +++ b/arch/arm/kernel/traps.c > >>>> @@ -188,9 +188,9 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) > >>>> } > >>>> } else { > >>>> if (thumb) > >>>> - bad = get_user(val, &((u16 *)addr)[i]); > >>>> + bad = get_user(val, &((u16 __user *)addr)[i]); > >>>> else > >>>> - bad = get_user(val, &((u32 *)addr)[i]); > >>>> + bad = get_user(val, &((u32 __user *)addr)[i]); > >>>> } > >>>> > >>>> if (!bad) > >>>> @@ -455,15 +455,15 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) > >>>> if (processor_mode(regs) == SVC_MODE) { > >>>> #ifdef CONFIG_THUMB2_KERNEL > >>>> if (thumb_mode(regs)) { > >>>> - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > >>>> + instr = __mem_to_opcode_thumb16(((__force u16 *)pc)[0]); > >>> > >>> Shouldn't this be __user as well? (and below) > >> > >> unsigned int instr; > >> void __user *pc; > >> > >> The __user can clear the warning, but a new warning will be generated. > >> > >> instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > >> ^new ^old > >> > >> arch/arm/kernel/traps.c:473:33: warning: dereference of noderef expression > >> > > > > This is because dereferencing a __user pointer is not permitted. > > > > So this code should be using get_kernel_nofault() here not a plain > > dereference of PC. So better to fix that properly instead of papering > > over it with a __force cast just to make sparse happy. > > How about: > @@ -451,9 +451,9 @@ int call_undef_hook(struct pt_regs *regs, unsigned int instr) > asmlinkage void do_undefinstr(struct pt_regs *regs) > { > unsigned int instr; > - void __user *pc; > + void *pc; > > - pc = (void __user *)instruction_pointer(regs); > + pc = (void *)instruction_pointer(regs); > > if (processor_mode(regs) == SVC_MODE) { > #ifdef CONFIG_THUMB2_KERNEL > @@ -497,7 +497,7 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) > } > #endif > arm_notify_die("Oops - undefined instruction", regs, > - SIGILL, ILL_ILLOPC, pc, 0, 6); > + SIGILL, ILL_ILLOPC, (void __user *)pc, 0, 6); > } > NOKPROBE_SYMBOL(do_undefinstr) > > > The 'pc' may come from kernel or user. And I found that all the get_user() > calls have already done type casts to the pc, except arm_notify_die(). > I think the above changes are reasonable. If we're going to do that, lets do it properly - I think the above would need some __force usage to stop sparse complaining, whereas I don't think this will (untested): diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c index 3f468ac98592..827cbc022900 100644 --- a/arch/arm/kernel/traps.c +++ b/arch/arm/kernel/traps.c @@ -449,36 +449,45 @@ int call_undef_hook(struct pt_regs *regs, unsigned int instr) asmlinkage void do_undefinstr(struct pt_regs *regs) { unsigned int instr; - void __user *pc; + unsigned long pc; - pc = (void __user *)instruction_pointer(regs); + pc = instruction_pointer(regs); if (processor_mode(regs) == SVC_MODE) { -#ifdef CONFIG_THUMB2_KERNEL - if (thumb_mode(regs)) { - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); + if (IS_ENABLED(CONFIG_THUMB2_KERNEL) && thumb_mode(regs)) { + u16 *tpc = (u16 *)pc; + + instr = __mem_to_opcode_thumb16(tpc[0]); if (is_wide_instruction(instr)) { u16 inst2; - inst2 = __mem_to_opcode_thumb16(((u16 *)pc)[1]); + + inst2 = __mem_to_opcode_thumb16(tpc[1]); instr = __opcode_thumb32_compose(instr, inst2); } - } else -#endif - instr = __mem_to_opcode_arm(*(u32 *) pc); + } else { + u32 *apc = (u32 *)pc; + + instr = __mem_to_opcode_arm(*apc); + } } else if (thumb_mode(regs)) { - if (get_user(instr, (u16 __user *)pc)) + u16 __user *tpc = (u16 __user *)pc; + + if (get_user(instr, tpc)) goto die_sig; instr = __mem_to_opcode_thumb16(instr); if (is_wide_instruction(instr)) { unsigned int instr2; - if (get_user(instr2, (u16 __user *)pc+1)) + if (get_user(instr2, tpc + 1)) goto die_sig; instr2 = __mem_to_opcode_thumb16(instr2); instr = __opcode_thumb32_compose(instr, instr2); } } else { - if (get_user(instr, (u32 __user *)pc)) + u32 __user *apc = (u32 __user *)pc; + + if (get_user(instr, apc)) goto die_sig; + instr = __mem_to_opcode_arm(instr); } @@ -495,7 +504,7 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) } #endif arm_notify_die("Oops - undefined instruction", regs, - SIGILL, ILL_ILLOPC, pc, 0, 6); + SIGILL, ILL_ILLOPC, (void __user *)pc, 0, 6); } NOKPROBE_SYMBOL(do_undefinstr)
On 2022/10/13 18:51, Russell King (Oracle) wrote: > On Tue, Oct 11, 2022 at 10:29:58AM +0800, Leizhen (ThunderTown) wrote: >> On 2022/10/10 19:06, Ard Biesheuvel wrote: >>> On Mon, 10 Oct 2022 at 12:58, Leizhen (ThunderTown) >>> <thunder.leizhen@huawei.com> wrote: >>>> On 2022/10/10 18:20, Ard Biesheuvel wrote: >>>>> On Mon, 10 Oct 2022 at 11:56, Zhen Lei <thunder.leizhen@huawei.com> wrote: >>>>>> >>>>>> Fix the following warnings: >>>>>> warning: incorrect type in initializer (different address spaces) >>>>>> expected unsigned short [noderef] __user *register __p >>>>>> got unsigned short [usertype] * >>>>>> warning: cast removes address space '__user' of expression >>>>>> >>>>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >>>>>> --- >>>>>> arch/arm/kernel/traps.c | 10 +++++----- >>>>>> 1 file changed, 5 insertions(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c >>>>>> index 20b2db6dcd1ced7..34aa80c09c508c1 100644 >>>>>> --- a/arch/arm/kernel/traps.c >>>>>> +++ b/arch/arm/kernel/traps.c >>>>>> @@ -188,9 +188,9 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) >>>>>> } >>>>>> } else { >>>>>> if (thumb) >>>>>> - bad = get_user(val, &((u16 *)addr)[i]); >>>>>> + bad = get_user(val, &((u16 __user *)addr)[i]); >>>>>> else >>>>>> - bad = get_user(val, &((u32 *)addr)[i]); >>>>>> + bad = get_user(val, &((u32 __user *)addr)[i]); >>>>>> } >>>>>> >>>>>> if (!bad) >>>>>> @@ -455,15 +455,15 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) >>>>>> if (processor_mode(regs) == SVC_MODE) { >>>>>> #ifdef CONFIG_THUMB2_KERNEL >>>>>> if (thumb_mode(regs)) { >>>>>> - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); >>>>>> + instr = __mem_to_opcode_thumb16(((__force u16 *)pc)[0]); >>>>> >>>>> Shouldn't this be __user as well? (and below) >>>> >>>> unsigned int instr; >>>> void __user *pc; >>>> >>>> The __user can clear the warning, but a new warning will be generated. >>>> >>>> instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); >>>> ^new ^old >>>> >>>> arch/arm/kernel/traps.c:473:33: warning: dereference of noderef expression >>>> >>> >>> This is because dereferencing a __user pointer is not permitted. >>> >>> So this code should be using get_kernel_nofault() here not a plain >>> dereference of PC. So better to fix that properly instead of papering >>> over it with a __force cast just to make sparse happy. >> >> How about: >> @@ -451,9 +451,9 @@ int call_undef_hook(struct pt_regs *regs, unsigned int instr) >> asmlinkage void do_undefinstr(struct pt_regs *regs) >> { >> unsigned int instr; >> - void __user *pc; >> + void *pc; >> >> - pc = (void __user *)instruction_pointer(regs); >> + pc = (void *)instruction_pointer(regs); >> >> if (processor_mode(regs) == SVC_MODE) { >> #ifdef CONFIG_THUMB2_KERNEL >> @@ -497,7 +497,7 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) >> } >> #endif >> arm_notify_die("Oops - undefined instruction", regs, >> - SIGILL, ILL_ILLOPC, pc, 0, 6); >> + SIGILL, ILL_ILLOPC, (void __user *)pc, 0, 6); >> } >> NOKPROBE_SYMBOL(do_undefinstr) >> >> >> The 'pc' may come from kernel or user. And I found that all the get_user() >> calls have already done type casts to the pc, except arm_notify_die(). >> I think the above changes are reasonable. > > If we're going to do that, lets do it properly - I think the above would > need some __force usage to stop sparse complaining, whereas I don't No need to add __force. I sent it after testing. $ cat .config | grep THUMB2 CONFIG_THUMB2_KERNEL=y $ make C=2 ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- arch/arm/kernel/traps.o CHECK scripts/mod/empty.c CALL scripts/checksyscalls.sh CHECK arch/arm/kernel/traps.c arch/arm/kernel/traps.c:97:6: warning: symbol 'dump_backtrace_stm' was not declared. Should it be static? arch/arm/kernel/traps.c:451:17: warning: symbol 'do_undefinstr' was not declared. Should it be static? arch/arm/kernel/traps.c:516:39: warning: symbol 'handle_fiq_as_nmi' was not declared. Should it be static? arch/arm/kernel/traps.c:535:17: warning: symbol 'bad_mode' was not declared. Should it be static? arch/arm/kernel/traps.c:608:16: warning: symbol 'arm_syscall' was not declared. Should it be static? arch/arm/kernel/traps.c:734:1: warning: symbol 'baddataabort' was not declared. Should it be static? arch/arm/kernel/traps.c:774:17: warning: symbol '__div0' was not declared. Should it be static? arch/arm/kernel/traps.c:781:6: warning: symbol 'abort' was not declared. Should it be static? arch/arm/kernel/traps.c:905:12: warning: symbol 'overflow_stack_ptr' was not declared. Should it be static? arch/arm/kernel/traps.c:922:17: warning: symbol 'handle_bad_stack' was not declared. Should it be static? > think this will (untested): > > diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > index 3f468ac98592..827cbc022900 100644 > --- a/arch/arm/kernel/traps.c > +++ b/arch/arm/kernel/traps.c > @@ -449,36 +449,45 @@ int call_undef_hook(struct pt_regs *regs, unsigned int instr) > asmlinkage void do_undefinstr(struct pt_regs *regs) > { > unsigned int instr; > - void __user *pc; > + unsigned long pc; > > - pc = (void __user *)instruction_pointer(regs); > + pc = instruction_pointer(regs); > > if (processor_mode(regs) == SVC_MODE) { > -#ifdef CONFIG_THUMB2_KERNEL > - if (thumb_mode(regs)) { > - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > + if (IS_ENABLED(CONFIG_THUMB2_KERNEL) && thumb_mode(regs)) { > + u16 *tpc = (u16 *)pc; > + > + instr = __mem_to_opcode_thumb16(tpc[0]); > if (is_wide_instruction(instr)) { > u16 inst2; > - inst2 = __mem_to_opcode_thumb16(((u16 *)pc)[1]); > + > + inst2 = __mem_to_opcode_thumb16(tpc[1]); > instr = __opcode_thumb32_compose(instr, inst2); > } > - } else > -#endif > - instr = __mem_to_opcode_arm(*(u32 *) pc); > + } else { > + u32 *apc = (u32 *)pc; > + > + instr = __mem_to_opcode_arm(*apc); > + } > } else if (thumb_mode(regs)) { > - if (get_user(instr, (u16 __user *)pc)) > + u16 __user *tpc = (u16 __user *)pc; > + > + if (get_user(instr, tpc)) > goto die_sig; > instr = __mem_to_opcode_thumb16(instr); > if (is_wide_instruction(instr)) { > unsigned int instr2; > - if (get_user(instr2, (u16 __user *)pc+1)) > + if (get_user(instr2, tpc + 1)) > goto die_sig; > instr2 = __mem_to_opcode_thumb16(instr2); > instr = __opcode_thumb32_compose(instr, instr2); > } > } else { > - if (get_user(instr, (u32 __user *)pc)) > + u32 __user *apc = (u32 __user *)pc; > + > + if (get_user(instr, apc)) > goto die_sig; > + > instr = __mem_to_opcode_arm(instr); > } > > @@ -495,7 +504,7 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) > } > #endif > arm_notify_die("Oops - undefined instruction", regs, > - SIGILL, ILL_ILLOPC, pc, 0, 6); > + SIGILL, ILL_ILLOPC, (void __user *)pc, 0, 6); > } > NOKPROBE_SYMBOL(do_undefinstr) > >
On 2022/10/13 18:51, Russell King (Oracle) wrote: > On Tue, Oct 11, 2022 at 10:29:58AM +0800, Leizhen (ThunderTown) wrote: >> On 2022/10/10 19:06, Ard Biesheuvel wrote: >>> On Mon, 10 Oct 2022 at 12:58, Leizhen (ThunderTown) >>> <thunder.leizhen@huawei.com> wrote: >>>> On 2022/10/10 18:20, Ard Biesheuvel wrote: >>>>> On Mon, 10 Oct 2022 at 11:56, Zhen Lei <thunder.leizhen@huawei.com> wrote: >>>>>> >>>>>> Fix the following warnings: >>>>>> warning: incorrect type in initializer (different address spaces) >>>>>> expected unsigned short [noderef] __user *register __p >>>>>> got unsigned short [usertype] * >>>>>> warning: cast removes address space '__user' of expression >>>>>> >>>>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >>>>>> --- >>>>>> arch/arm/kernel/traps.c | 10 +++++----- >>>>>> 1 file changed, 5 insertions(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c >>>>>> index 20b2db6dcd1ced7..34aa80c09c508c1 100644 >>>>>> --- a/arch/arm/kernel/traps.c >>>>>> +++ b/arch/arm/kernel/traps.c >>>>>> @@ -188,9 +188,9 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) >>>>>> } >>>>>> } else { >>>>>> if (thumb) >>>>>> - bad = get_user(val, &((u16 *)addr)[i]); >>>>>> + bad = get_user(val, &((u16 __user *)addr)[i]); >>>>>> else >>>>>> - bad = get_user(val, &((u32 *)addr)[i]); >>>>>> + bad = get_user(val, &((u32 __user *)addr)[i]); >>>>>> } >>>>>> >>>>>> if (!bad) >>>>>> @@ -455,15 +455,15 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) >>>>>> if (processor_mode(regs) == SVC_MODE) { >>>>>> #ifdef CONFIG_THUMB2_KERNEL >>>>>> if (thumb_mode(regs)) { >>>>>> - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); >>>>>> + instr = __mem_to_opcode_thumb16(((__force u16 *)pc)[0]); >>>>> >>>>> Shouldn't this be __user as well? (and below) >>>> >>>> unsigned int instr; >>>> void __user *pc; >>>> >>>> The __user can clear the warning, but a new warning will be generated. >>>> >>>> instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); >>>> ^new ^old >>>> >>>> arch/arm/kernel/traps.c:473:33: warning: dereference of noderef expression >>>> >>> >>> This is because dereferencing a __user pointer is not permitted. >>> >>> So this code should be using get_kernel_nofault() here not a plain >>> dereference of PC. So better to fix that properly instead of papering >>> over it with a __force cast just to make sparse happy. >> >> How about: >> @@ -451,9 +451,9 @@ int call_undef_hook(struct pt_regs *regs, unsigned int instr) >> asmlinkage void do_undefinstr(struct pt_regs *regs) >> { >> unsigned int instr; >> - void __user *pc; >> + void *pc; >> >> - pc = (void __user *)instruction_pointer(regs); >> + pc = (void *)instruction_pointer(regs); >> >> if (processor_mode(regs) == SVC_MODE) { >> #ifdef CONFIG_THUMB2_KERNEL >> @@ -497,7 +497,7 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) >> } >> #endif >> arm_notify_die("Oops - undefined instruction", regs, >> - SIGILL, ILL_ILLOPC, pc, 0, 6); >> + SIGILL, ILL_ILLOPC, (void __user *)pc, 0, 6); >> } >> NOKPROBE_SYMBOL(do_undefinstr) >> >> >> The 'pc' may come from kernel or user. And I found that all the get_user() >> calls have already done type casts to the pc, except arm_notify_die(). >> I think the above changes are reasonable. > > If we're going to do that, lets do it properly - I think the above would > need some __force usage to stop sparse complaining, whereas I don't > think this will (untested): The following changes are a little too big, and the above changes have solved the problem completely. In order to focus on the main target, I removed this part of the modification in v4. I hope you'll take the time to review v4. > > diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > index 3f468ac98592..827cbc022900 100644 > --- a/arch/arm/kernel/traps.c > +++ b/arch/arm/kernel/traps.c > @@ -449,36 +449,45 @@ int call_undef_hook(struct pt_regs *regs, unsigned int instr) > asmlinkage void do_undefinstr(struct pt_regs *regs) > { > unsigned int instr; > - void __user *pc; > + unsigned long pc; The following changes need to be added: @@ -487,7 +497,7 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) die_sig: #ifdef CONFIG_DEBUG_USER if (user_debug & UDBG_UNDEFINED) { - pr_info("%s (%d): undefined instruction: pc=%px\n", + pr_info("%s (%d): undefined instruction: pc=%lx\n", current->comm, task_pid_nr(current), pc); __show_regs(regs); dump_instr(KERN_INFO, regs); > > - pc = (void __user *)instruction_pointer(regs); > + pc = instruction_pointer(regs); > > if (processor_mode(regs) == SVC_MODE) { > -#ifdef CONFIG_THUMB2_KERNEL > - if (thumb_mode(regs)) { > - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > + if (IS_ENABLED(CONFIG_THUMB2_KERNEL) && thumb_mode(regs)) { > + u16 *tpc = (u16 *)pc; > + > + instr = __mem_to_opcode_thumb16(tpc[0]); > if (is_wide_instruction(instr)) { > u16 inst2; > - inst2 = __mem_to_opcode_thumb16(((u16 *)pc)[1]); > + > + inst2 = __mem_to_opcode_thumb16(tpc[1]); > instr = __opcode_thumb32_compose(instr, inst2); > } > - } else > -#endif > - instr = __mem_to_opcode_arm(*(u32 *) pc); > + } else { > + u32 *apc = (u32 *)pc; > + > + instr = __mem_to_opcode_arm(*apc); > + } > } else if (thumb_mode(regs)) { > - if (get_user(instr, (u16 __user *)pc)) > + u16 __user *tpc = (u16 __user *)pc; > + > + if (get_user(instr, tpc)) > goto die_sig; > instr = __mem_to_opcode_thumb16(instr); > if (is_wide_instruction(instr)) { > unsigned int instr2; > - if (get_user(instr2, (u16 __user *)pc+1)) > + if (get_user(instr2, tpc + 1)) > goto die_sig; > instr2 = __mem_to_opcode_thumb16(instr2); > instr = __opcode_thumb32_compose(instr, instr2); > } > } else { > - if (get_user(instr, (u32 __user *)pc)) > + u32 __user *apc = (u32 __user *)pc; > + > + if (get_user(instr, apc)) > goto die_sig; > + > instr = __mem_to_opcode_arm(instr); > } > > @@ -495,7 +504,7 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) > } > #endif > arm_notify_die("Oops - undefined instruction", regs, > - SIGILL, ILL_ILLOPC, pc, 0, 6); > + SIGILL, ILL_ILLOPC, (void __user *)pc, 0, 6); > } > NOKPROBE_SYMBOL(do_undefinstr) > >
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c index 20b2db6dcd1ced7..34aa80c09c508c1 100644 --- a/arch/arm/kernel/traps.c +++ b/arch/arm/kernel/traps.c @@ -188,9 +188,9 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) } } else { if (thumb) - bad = get_user(val, &((u16 *)addr)[i]); + bad = get_user(val, &((u16 __user *)addr)[i]); else - bad = get_user(val, &((u32 *)addr)[i]); + bad = get_user(val, &((u32 __user *)addr)[i]); } if (!bad) @@ -455,15 +455,15 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) if (processor_mode(regs) == SVC_MODE) { #ifdef CONFIG_THUMB2_KERNEL if (thumb_mode(regs)) { - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); + instr = __mem_to_opcode_thumb16(((__force u16 *)pc)[0]); if (is_wide_instruction(instr)) { u16 inst2; - inst2 = __mem_to_opcode_thumb16(((u16 *)pc)[1]); + inst2 = __mem_to_opcode_thumb16(((__force u16 *)pc)[1]); instr = __opcode_thumb32_compose(instr, inst2); } } else #endif - instr = __mem_to_opcode_arm(*(u32 *) pc); + instr = __mem_to_opcode_arm(*(__force u32 *) pc); } else if (thumb_mode(regs)) { if (get_user(instr, (u16 __user *)pc)) goto die_sig;
Fix the following warnings: warning: incorrect type in initializer (different address spaces) expected unsigned short [noderef] __user *register __p got unsigned short [usertype] * warning: cast removes address space '__user' of expression Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> --- arch/arm/kernel/traps.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)