[PATCH v2] riscv: Ensure the value of FP registers in the core dump file is up to date

[PATCH v2] riscv: Ensure the value of FP registers in the core dump file is up to date

[Vincent Chen]

The value of FP registers in the core dump file comes from the
thread.fstate. However, kernel saves the FP registers to the thread.fstate
only before scheduling out the process. If no process switch happens
during the exception handling process, kernel will not have a chance to
save the latest value of FP registers to thread.fstate. It will cause the
value of FP registers in the core dump file may be incorrect. To solve this
problem, this patch force lets kernel save the FP register into the
thread.fstate if the target task_struct equals the current.

Signed-off-by: Vincent Chen <vincent.chen@sifive.com>

---
Changes since the v1 patch
1. Include switch_to.h to avoid compiling error.

---
 arch/riscv/kernel/ptrace.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
index 1a85305720e8..9c0511119bad 100644
--- a/arch/riscv/kernel/ptrace.c
+++ b/arch/riscv/kernel/ptrace.c
@@ -10,6 +10,7 @@
 #include <asm/ptrace.h>
 #include <asm/syscall.h>
 #include <asm/thread_info.h>
+#include <asm/switch_to.h>
 #include <linux/audit.h>
 #include <linux/ptrace.h>
 #include <linux/elf.h>
@@ -56,6 +57,9 @@ static int riscv_fpr_get(struct task_struct *target,
 {
 	struct __riscv_d_ext_state *fstate = &target->thread.fstate;
 
+	if (target == current)
+		fstate_save(current, task_pt_regs(current));
+
 	membuf_write(&to, fstate, offsetof(struct __riscv_d_ext_state, fcsr));
 	membuf_store(&to, fstate->fcsr);
 	return membuf_zero(&to, 4);	// explicitly pad
-- 
2.32.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv: Ensure the value of FP registers in the core dump file is up to date

[Jisheng Zhang]

On Tue,  3 Aug 2021 17:27:51 +0800
Vincent Chen <vincent.chen@sifive.com> wrote:

> The value of FP registers in the core dump file comes from the
> thread.fstate. However, kernel saves the FP registers to the thread.fstate
> only before scheduling out the process. If no process switch happens
> during the exception handling process, kernel will not have a chance to
> save the latest value of FP registers to thread.fstate. It will cause the
> value of FP registers in the core dump file may be incorrect. To solve this
> problem, this patch force lets kernel save the FP register into the
> thread.fstate if the target task_struct equals the current.

Nice catch! I think there's another case too: since last kernel save, the FP
registers has been modified by the userspace, we also need to force fstate_save()
This patch fixes both cases.

> 
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>

Reviewed-by: Jisheng Zhang <jszhang@kernel.org>

> 
> ---
> Changes since the v1 patch
> 1. Include switch_to.h to avoid compiling error.
> 
> ---
>  arch/riscv/kernel/ptrace.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
> index 1a85305720e8..9c0511119bad 100644
> --- a/arch/riscv/kernel/ptrace.c
> +++ b/arch/riscv/kernel/ptrace.c
> @@ -10,6 +10,7 @@
>  #include <asm/ptrace.h>
>  #include <asm/syscall.h>
>  #include <asm/thread_info.h>
> +#include <asm/switch_to.h>
>  #include <linux/audit.h>
>  #include <linux/ptrace.h>
>  #include <linux/elf.h>
> @@ -56,6 +57,9 @@ static int riscv_fpr_get(struct task_struct *target,
>  {
>  	struct __riscv_d_ext_state *fstate = &target->thread.fstate;
>  
> +	if (target == current)
> +		fstate_save(current, task_pt_regs(current));
> +
>  	membuf_write(&to, fstate, offsetof(struct __riscv_d_ext_state, fcsr));
>  	membuf_store(&to, fstate->fcsr);
>  	return membuf_zero(&to, 4);	// explicitly pad



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] riscv: Ensure the value of FP registers in the core dump file is up to date

[Palmer Dabbelt]

On Tue, 03 Aug 2021 02:27:51 PDT (-0700), vincent.chen@sifive.com wrote:
> The value of FP registers in the core dump file comes from the
> thread.fstate. However, kernel saves the FP registers to the thread.fstate
> only before scheduling out the process. If no process switch happens
> during the exception handling process, kernel will not have a chance to
> save the latest value of FP registers to thread.fstate. It will cause the
> value of FP registers in the core dump file may be incorrect. To solve this
> problem, this patch force lets kernel save the FP register into the
> thread.fstate if the target task_struct equals the current.
>
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
>
> ---
> Changes since the v1 patch
> 1. Include switch_to.h to avoid compiling error.
>
> ---
>  arch/riscv/kernel/ptrace.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
> index 1a85305720e8..9c0511119bad 100644
> --- a/arch/riscv/kernel/ptrace.c
> +++ b/arch/riscv/kernel/ptrace.c
> @@ -10,6 +10,7 @@
>  #include <asm/ptrace.h>
>  #include <asm/syscall.h>
>  #include <asm/thread_info.h>
> +#include <asm/switch_to.h>
>  #include <linux/audit.h>
>  #include <linux/ptrace.h>
>  #include <linux/elf.h>
> @@ -56,6 +57,9 @@ static int riscv_fpr_get(struct task_struct *target,
>  {
>  	struct __riscv_d_ext_state *fstate = &target->thread.fstate;
>
> +	if (target == current)
> +		fstate_save(current, task_pt_regs(current));
> +
>  	membuf_write(&to, fstate, offsetof(struct __riscv_d_ext_state, fcsr));
>  	membuf_store(&to, fstate->fcsr);
>  	return membuf_zero(&to, 4);	// explicitly pad

Thanks, this is on fixes.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv