Annotating a pointer from kernel to __user and then back again might
confuse sparse. In call_undef_hook() it can be avoided by not using the
intermediate user pointer variable.
Note: This patch adds no functional changes to code.
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Amit Daniel Kachhap <[email protected]>
---
arch/arm64/kernel/traps.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index b03e383d944a..357d10a8bbf5 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -404,7 +404,8 @@ static int call_undef_hook(struct pt_regs *regs)
if (!user_mode(regs)) {
__le32 instr_le;
- if (get_kernel_nofault(instr_le, (__force __le32 *)pc))
+ if (get_kernel_nofault(instr_le,
+ (__le32 *)instruction_pointer(regs)))
goto exit;
instr = le32_to_cpu(instr_le);
} else if (compat_thumb_mode(regs)) {
--
2.17.1
On Tue, Sep 14, 2021 at 08:57:42PM +0530, Amit Daniel Kachhap wrote:
> Annotating a pointer from kernel to __user and then back again might
> confuse sparse. In call_undef_hook() it can be avoided by not using the
> intermediate user pointer variable.
When you say "might confuse sparse", does it complain today? If so, can
you include an example of what goes wrong?
> Note: This patch adds no functional changes to code.
>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Amit Daniel Kachhap <[email protected]>
> ---
> arch/arm64/kernel/traps.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index b03e383d944a..357d10a8bbf5 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -404,7 +404,8 @@ static int call_undef_hook(struct pt_regs *regs)
>
> if (!user_mode(regs)) {
> __le32 instr_le;
> - if (get_kernel_nofault(instr_le, (__force __le32 *)pc))
> + if (get_kernel_nofault(instr_le,
> + (__le32 *)instruction_pointer(regs)))
Can we make `pc` an unsigned long, instead?
It'd be nice to handle all three cases consistently, even if that means
adding __force to the two user cases.
Thanks,
Mark.
> goto exit;
> instr = le32_to_cpu(instr_le);
> } else if (compat_thumb_mode(regs)) {
> --
> 2.17.1
>
Hi,
On 9/14/21 9:30 PM, Mark Rutland wrote:
> On Tue, Sep 14, 2021 at 08:57:42PM +0530, Amit Daniel Kachhap wrote:
>> Annotating a pointer from kernel to __user and then back again might
>> confuse sparse. In call_undef_hook() it can be avoided by not using the
>> intermediate user pointer variable.
>
> When you say "might confuse sparse", does it complain today? If so, can
> you include an example of what goes wrong?
No it does not give warning. The __force option silences the warning. My
idea is to remove the unwanted __force annotations and not mix user and
kernel pointers.
>
>> Note: This patch adds no functional changes to code.
>>
>> Cc: Catalin Marinas <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Signed-off-by: Amit Daniel Kachhap <[email protected]>
>> ---
>> arch/arm64/kernel/traps.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>> index b03e383d944a..357d10a8bbf5 100644
>> --- a/arch/arm64/kernel/traps.c
>> +++ b/arch/arm64/kernel/traps.c
>> @@ -404,7 +404,8 @@ static int call_undef_hook(struct pt_regs *regs)
>>
>> if (!user_mode(regs)) {
>> __le32 instr_le;
>> - if (get_kernel_nofault(instr_le, (__force __le32 *)pc))
>> + if (get_kernel_nofault(instr_le,
>> + (__le32 *)instruction_pointer(regs)))
>
> Can we make `pc` an unsigned long, instead?
I think it can be done.
>
> It'd be nice to handle all three cases consistently, even if that means
> adding __force to the two user cases.
Agree with your suggestion. Even in the 2 user cases, __force may not be
needed as the typecast will be from from unsigned long to user pointer.
BR,
Amit
>
> Thanks,
> Mark.
>
>> goto exit;
>> instr = le32_to_cpu(instr_le);
>> } else if (compat_thumb_mode(regs)) {
>> --
>> 2.17.1
>>