* [PATCH] LoongArch: Fix missing fcsr in ptrace's fpr_set
@ 2022-07-14 6:25 Qi Hu
2022-07-14 6:51 ` Conor.Dooley
2022-07-14 13:12 ` Huacai Chen
0 siblings, 2 replies; 6+ messages in thread
From: Qi Hu @ 2022-07-14 6:25 UTC (permalink / raw)
To: Huacai Chen, WANG Xuerui, Oleg Nesterov; +Cc: Xu Li, loongarch, LKML, Qi Hu
In file ptrace.c, function fpr_set does not copy fcsr data from ubuf
to kbuf. That's the reason why fcsr cannot be modified by ptrace.
This patch fixs this problem and allows users using ptrace to modify
the fcsr.
Signed-off-by: Qi Hu <huqi@loongson.cn>
Signed-off-by: Xu Li <lixu@loongson.cn>
---
arch/loongarch/kernel/ptrace.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/loongarch/kernel/ptrace.c b/arch/loongarch/kernel/ptrace.c
index e6ab87948e1d..dc2b82ea894c 100644
--- a/arch/loongarch/kernel/ptrace.c
+++ b/arch/loongarch/kernel/ptrace.c
@@ -193,7 +193,7 @@ static int fpr_set(struct task_struct *target,
const void *kbuf, const void __user *ubuf)
{
const int fcc_start = NUM_FPU_REGS * sizeof(elf_fpreg_t);
- const int fcc_end = fcc_start + sizeof(u64);
+ const int fcsr_start = fcc_start + sizeof(u64);
int err;
BUG_ON(count % sizeof(elf_fpreg_t));
@@ -209,10 +209,12 @@ static int fpr_set(struct task_struct *target,
if (err)
return err;
- if (count > 0)
- err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
- &target->thread.fpu.fcc,
- fcc_start, fcc_end);
+ err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.fpu.fcc, fcc_start,
+ fcc_start + sizeof(u64));
+ err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.fpu.fcsr, fcsr_start,
+ fcsr_start + sizeof(u32));
return err;
}
--
2.37.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] LoongArch: Fix missing fcsr in ptrace's fpr_set
2022-07-14 6:25 [PATCH] LoongArch: Fix missing fcsr in ptrace's fpr_set Qi Hu
@ 2022-07-14 6:51 ` Conor.Dooley
2022-07-14 7:14 ` Qi Hu
2022-07-14 13:12 ` Huacai Chen
1 sibling, 1 reply; 6+ messages in thread
From: Conor.Dooley @ 2022-07-14 6:51 UTC (permalink / raw)
To: huqi, chenhuacai, kernel, oleg; +Cc: lixu, loongarch, linux-kernel
On 14/07/2022 07:25, Qi Hu wrote:
> In file ptrace.c, function fpr_set does not copy fcsr data from ubuf
> to kbuf. That's the reason why fcsr cannot be modified by ptrace.
>
> This patch fixs this problem and allows users using ptrace to modify
> the fcsr.
>
> Signed-off-by: Qi Hu <huqi@loongson.cn>
> Signed-off-by: Xu Li <lixu@loongson.cn>
Hey Qi Hu,
Why does this have Xu Li's SoB if they're neither the author nor listed
as a Co-developed-by:?
As submitter, your SoB should be last anyway.
Thanks,
Conor.
> ---
> arch/loongarch/kernel/ptrace.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/loongarch/kernel/ptrace.c b/arch/loongarch/kernel/ptrace.c
> index e6ab87948e1d..dc2b82ea894c 100644
> --- a/arch/loongarch/kernel/ptrace.c
> +++ b/arch/loongarch/kernel/ptrace.c
> @@ -193,7 +193,7 @@ static int fpr_set(struct task_struct *target,
> const void *kbuf, const void __user *ubuf)
> {
> const int fcc_start = NUM_FPU_REGS * sizeof(elf_fpreg_t);
> - const int fcc_end = fcc_start + sizeof(u64);
> + const int fcsr_start = fcc_start + sizeof(u64);
> int err;
>
> BUG_ON(count % sizeof(elf_fpreg_t));
> @@ -209,10 +209,12 @@ static int fpr_set(struct task_struct *target,
> if (err)
> return err;
>
> - if (count > 0)
> - err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> - &target->thread.fpu.fcc,
> - fcc_start, fcc_end);
> + err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> + &target->thread.fpu.fcc, fcc_start,
> + fcc_start + sizeof(u64));
> + err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> + &target->thread.fpu.fcsr, fcsr_start,
> + fcsr_start + sizeof(u32));
>
> return err;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] LoongArch: Fix missing fcsr in ptrace's fpr_set
2022-07-14 6:51 ` Conor.Dooley
@ 2022-07-14 7:14 ` Qi Hu
0 siblings, 0 replies; 6+ messages in thread
From: Qi Hu @ 2022-07-14 7:14 UTC (permalink / raw)
To: Conor.Dooley, chenhuacai, kernel, oleg; +Cc: lixu, loongarch, linux-kernel
On 2022/7/14 14:51, Conor.Dooley@microchip.com wrote:
> On 14/07/2022 07:25, Qi Hu wrote:
>> In file ptrace.c, function fpr_set does not copy fcsr data from ubuf
>> to kbuf. That's the reason why fcsr cannot be modified by ptrace.
>>
>> This patch fixs this problem and allows users using ptrace to modify
>> the fcsr.
>>
>> Signed-off-by: Qi Hu <huqi@loongson.cn>
>> Signed-off-by: Xu Li <lixu@loongson.cn>
> Hey Qi Hu,
>
> Why does this have Xu Li's SoB if they're neither the author nor listed
> as a Co-developed-by:?
>
> As submitter, your SoB should be last anyway.
> Thanks,
> Conor.
>
Ops. I realize using SoB here is incorrect. I will change it into
Co-developed-by.
Thanks.
Qi.
>> ---
>> arch/loongarch/kernel/ptrace.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/loongarch/kernel/ptrace.c b/arch/loongarch/kernel/ptrace.c
>> index e6ab87948e1d..dc2b82ea894c 100644
>> --- a/arch/loongarch/kernel/ptrace.c
>> +++ b/arch/loongarch/kernel/ptrace.c
>> @@ -193,7 +193,7 @@ static int fpr_set(struct task_struct *target,
>> const void *kbuf, const void __user *ubuf)
>> {
>> const int fcc_start = NUM_FPU_REGS * sizeof(elf_fpreg_t);
>> - const int fcc_end = fcc_start + sizeof(u64);
>> + const int fcsr_start = fcc_start + sizeof(u64);
>> int err;
>>
>> BUG_ON(count % sizeof(elf_fpreg_t));
>> @@ -209,10 +209,12 @@ static int fpr_set(struct task_struct *target,
>> if (err)
>> return err;
>>
>> - if (count > 0)
>> - err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
>> - &target->thread.fpu.fcc,
>> - fcc_start, fcc_end);
>> + err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
>> + &target->thread.fpu.fcc, fcc_start,
>> + fcc_start + sizeof(u64));
>> + err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
>> + &target->thread.fpu.fcsr, fcsr_start,
>> + fcsr_start + sizeof(u32));
>>
>> return err;
>> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] LoongArch: Fix missing fcsr in ptrace's fpr_set
2022-07-14 6:25 [PATCH] LoongArch: Fix missing fcsr in ptrace's fpr_set Qi Hu
2022-07-14 6:51 ` Conor.Dooley
@ 2022-07-14 13:12 ` Huacai Chen
2022-07-14 14:07 ` Qi Hu
1 sibling, 1 reply; 6+ messages in thread
From: Huacai Chen @ 2022-07-14 13:12 UTC (permalink / raw)
To: Qi Hu; +Cc: WANG Xuerui, Oleg Nesterov, Xu Li, loongarch, LKML
Hi,
On Thu, Jul 14, 2022 at 2:26 PM Qi Hu <huqi@loongson.cn> wrote:
>
> In file ptrace.c, function fpr_set does not copy fcsr data from ubuf
> to kbuf. That's the reason why fcsr cannot be modified by ptrace.
>
> This patch fixs this problem and allows users using ptrace to modify
> the fcsr.
>
> Signed-off-by: Qi Hu <huqi@loongson.cn>
> Signed-off-by: Xu Li <lixu@loongson.cn>
> ---
> arch/loongarch/kernel/ptrace.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/loongarch/kernel/ptrace.c b/arch/loongarch/kernel/ptrace.c
> index e6ab87948e1d..dc2b82ea894c 100644
> --- a/arch/loongarch/kernel/ptrace.c
> +++ b/arch/loongarch/kernel/ptrace.c
> @@ -193,7 +193,7 @@ static int fpr_set(struct task_struct *target,
> const void *kbuf, const void __user *ubuf)
> {
> const int fcc_start = NUM_FPU_REGS * sizeof(elf_fpreg_t);
> - const int fcc_end = fcc_start + sizeof(u64);
> + const int fcsr_start = fcc_start + sizeof(u64);
> int err;
>
> BUG_ON(count % sizeof(elf_fpreg_t));
> @@ -209,10 +209,12 @@ static int fpr_set(struct task_struct *target,
> if (err)
> return err;
>
> - if (count > 0)
> - err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> - &target->thread.fpu.fcc,
> - fcc_start, fcc_end);
> + err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> + &target->thread.fpu.fcc, fcc_start,
> + fcc_start + sizeof(u64));
> + err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> + &target->thread.fpu.fcsr, fcsr_start,
> + fcsr_start + sizeof(u32));
You shouldn't remove (count > 0) here, because the above
user_regset_copyin() will modify count inside, and so "count == 0" is
possible.
Huacai
>
> return err;
> }
> --
> 2.37.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] LoongArch: Fix missing fcsr in ptrace's fpr_set
2022-07-14 13:12 ` Huacai Chen
@ 2022-07-14 14:07 ` Qi Hu
2022-07-15 3:20 ` Huacai Chen
0 siblings, 1 reply; 6+ messages in thread
From: Qi Hu @ 2022-07-14 14:07 UTC (permalink / raw)
To: Huacai Chen; +Cc: WANG Xuerui, Oleg Nesterov, Xu Li, loongarch, LKML
On 2022/7/14 21:12, Huacai Chen wrote:
> Hi,
>
> On Thu, Jul 14, 2022 at 2:26 PM Qi Hu <huqi@loongson.cn> wrote:
>> In file ptrace.c, function fpr_set does not copy fcsr data from ubuf
>> to kbuf. That's the reason why fcsr cannot be modified by ptrace.
>>
>> This patch fixs this problem and allows users using ptrace to modify
>> the fcsr.
>>
>> Signed-off-by: Qi Hu <huqi@loongson.cn>
>> Signed-off-by: Xu Li <lixu@loongson.cn>
>> ---
>> arch/loongarch/kernel/ptrace.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/loongarch/kernel/ptrace.c b/arch/loongarch/kernel/ptrace.c
>> index e6ab87948e1d..dc2b82ea894c 100644
>> --- a/arch/loongarch/kernel/ptrace.c
>> +++ b/arch/loongarch/kernel/ptrace.c
>> @@ -193,7 +193,7 @@ static int fpr_set(struct task_struct *target,
>> const void *kbuf, const void __user *ubuf)
>> {
>> const int fcc_start = NUM_FPU_REGS * sizeof(elf_fpreg_t);
>> - const int fcc_end = fcc_start + sizeof(u64);
>> + const int fcsr_start = fcc_start + sizeof(u64);
>> int err;
>>
>> BUG_ON(count % sizeof(elf_fpreg_t));
>> @@ -209,10 +209,12 @@ static int fpr_set(struct task_struct *target,
>> if (err)
>> return err;
>>
>> - if (count > 0)
>> - err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
>> - &target->thread.fpu.fcc,
>> - fcc_start, fcc_end);
>> + err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
>> + &target->thread.fpu.fcc, fcc_start,
>> + fcc_start + sizeof(u64));
>> + err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
>> + &target->thread.fpu.fcsr, fcsr_start,
>> + fcsr_start + sizeof(u32));
> You shouldn't remove (count > 0) here, because the above
> user_regset_copyin() will modify count inside, and so "count == 0" is
> possible.
>
> Huacai
Yes, the "count" should be considered. But the "count" is checked at the
beginning of the "user_regset_copyin()".
So "count > 0" is useless, i think.
Also like riscv, "count" is not checked in "riscv_fpr_set()".
>> return err;
>> }
>> --
>> 2.37.0
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] LoongArch: Fix missing fcsr in ptrace's fpr_set
2022-07-14 14:07 ` Qi Hu
@ 2022-07-15 3:20 ` Huacai Chen
0 siblings, 0 replies; 6+ messages in thread
From: Huacai Chen @ 2022-07-15 3:20 UTC (permalink / raw)
To: Qi Hu; +Cc: WANG Xuerui, Oleg Nesterov, Xu Li, loongarch, LKML
On Thu, Jul 14, 2022 at 10:07 PM Qi Hu <huqi@loongson.cn> wrote:
>
>
> On 2022/7/14 21:12, Huacai Chen wrote:
> > Hi,
> >
> > On Thu, Jul 14, 2022 at 2:26 PM Qi Hu <huqi@loongson.cn> wrote:
> >> In file ptrace.c, function fpr_set does not copy fcsr data from ubuf
> >> to kbuf. That's the reason why fcsr cannot be modified by ptrace.
> >>
> >> This patch fixs this problem and allows users using ptrace to modify
> >> the fcsr.
> >>
> >> Signed-off-by: Qi Hu <huqi@loongson.cn>
> >> Signed-off-by: Xu Li <lixu@loongson.cn>
> >> ---
> >> arch/loongarch/kernel/ptrace.c | 12 +++++++-----
> >> 1 file changed, 7 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch/loongarch/kernel/ptrace.c b/arch/loongarch/kernel/ptrace.c
> >> index e6ab87948e1d..dc2b82ea894c 100644
> >> --- a/arch/loongarch/kernel/ptrace.c
> >> +++ b/arch/loongarch/kernel/ptrace.c
> >> @@ -193,7 +193,7 @@ static int fpr_set(struct task_struct *target,
> >> const void *kbuf, const void __user *ubuf)
> >> {
> >> const int fcc_start = NUM_FPU_REGS * sizeof(elf_fpreg_t);
> >> - const int fcc_end = fcc_start + sizeof(u64);
> >> + const int fcsr_start = fcc_start + sizeof(u64);
> >> int err;
> >>
> >> BUG_ON(count % sizeof(elf_fpreg_t));
> >> @@ -209,10 +209,12 @@ static int fpr_set(struct task_struct *target,
> >> if (err)
> >> return err;
> >>
> >> - if (count > 0)
> >> - err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> >> - &target->thread.fpu.fcc,
> >> - fcc_start, fcc_end);
> >> + err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> >> + &target->thread.fpu.fcc, fcc_start,
> >> + fcc_start + sizeof(u64));
> >> + err |= user_regset_copyin(&pos, &count, &kbuf, &ubuf,
> >> + &target->thread.fpu.fcsr, fcsr_start,
> >> + fcsr_start + sizeof(u32));
> > You shouldn't remove (count > 0) here, because the above
> > user_regset_copyin() will modify count inside, and so "count == 0" is
> > possible.
> >
> > Huacai
>
> Yes, the "count" should be considered. But the "count" is checked at the
> beginning of the "user_regset_copyin()".
>
> So "count > 0" is useless, i think.
Yes, you are right, thanks.
Huacai
>
> Also like riscv, "count" is not checked in "riscv_fpr_set()".
>
> >> return err;
> >> }
> >> --
> >> 2.37.0
> >>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-15 3:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-14 6:25 [PATCH] LoongArch: Fix missing fcsr in ptrace's fpr_set Qi Hu
2022-07-14 6:51 ` Conor.Dooley
2022-07-14 7:14 ` Qi Hu
2022-07-14 13:12 ` Huacai Chen
2022-07-14 14:07 ` Qi Hu
2022-07-15 3:20 ` Huacai Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®