Skip to contentSkip to conversation

arm64: fix missing __user in compat_vfp_{get,set}()

PATCH New Luc Van Oostenryck 3 participants 7 messages Jun 28, 2017 – Jun 29, 2017

Subsystems (2)
arm64 ptrace support
lore
Share & tools
lore

Luc Van Oostenryck

author
#0 lore raw

PATCH 1/1 arm64: fix missing __user in compat_vfp_{get,set}()

compat_vfp_get() & compat_vfp_set() are two helpers reading or writting some values via {get,put}_user() which need a pointer annotated with '__user'.
The buffers used by the two helpers are correctly '__user' annotated but need to be casted to a real type before being given to {get,put}_user().

The problem is that this cast lack a '__user' annotation.

Fix this by adding the missing '__user'.

Signed-off-by Luc Van Oostenryck <[email protected]>
---
 arch/arm64/kernel/ptrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/ptrace.c +2 -2
index c142459a8..c77f425c7 100644
@@ -908,7 +908,7 @@ static int compat_vfp_get(struct task_struct *target,
 	if (count && !ret) {
 		fpscr = (uregs->fpsr & VFP_FPSCR_STAT_MASK) |
 			(uregs->fpcr & VFP_FPSCR_CTRL_MASK);
-		ret = put_user(fpscr, (compat_ulong_t *)ubuf);
+		ret = put_user(fpscr, (compat_ulong_t __user *)ubuf);
 	}
 
 	return ret;

@@ -932,7 +932,7 @@ static int compat_vfp_set(struct task_struct *target,
 				 VFP_STATE_SIZE - sizeof(compat_ulong_t));
 
 	if (count && !ret) {
-		ret = get_user(fpscr, (compat_ulong_t *)ubuf);
+		ret = get_user(fpscr, (compat_ulong_t __user *)ubuf);
 		uregs->fpsr = fpscr & VFP_FPSCR_STAT_MASK;
 		uregs->fpcr = fpscr & VFP_FPSCR_CTRL_MASK;
 	}
Signature · 1 line
2.13.0
#1 lore raw

PATCH 1/1 arm64: fix missing __user in compat_vfp_{get,set}()

Quoted 10 lines · Luc Van Oostenryck

compat_vfp_get() & compat_vfp_set() are two helpers reading or writting some values via {get,put}_user() which need a pointer annotated with '__user'.
The buffers used by the two helpers are correctly '__user' annotated but need to be casted to a real type before being given to {get,put}_user().

The problem is that this cast lack a '__user' annotation.

Fix this by adding the missing '__user'.

Casting __user off is something that sparse certainly should be warning about (and it was, by the looks of it).

Reviewed-by Dave Martin <[email protected]>

(There remains a sparse warning in compat_ptrace_write_user(), where we pass implicitly cast __user onto a pointer, under set_fs(KERNEL_DS). I'm not sure what the most correct fix is there.)

Cheers
---Dave

Quoted 36 lines
Signed-off-by Luc Van Oostenryck <[email protected]>
---
 arch/arm64/kernel/ptrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kernel/ptrace.c +2 -2
index c142459a8..c77f425c7 100644
@@ -908,7 +908,7 @@ static int compat_vfp_get(struct task_struct *target,
 	if (count && !ret) {
 		fpscr = (uregs->fpsr & VFP_FPSCR_STAT_MASK) |
 			(uregs->fpcr & VFP_FPSCR_CTRL_MASK);
-		ret = put_user(fpscr, (compat_ulong_t *)ubuf);
+		ret = put_user(fpscr, (compat_ulong_t __user *)ubuf);
 	}
 
 	return ret;

@@ -932,7 +932,7 @@ static int compat_vfp_set(struct task_struct *target,
 				 VFP_STATE_SIZE - sizeof(compat_ulong_t));
 
 	if (count && !ret) {
-		ret = get_user(fpscr, (compat_ulong_t *)ubuf);
+		ret = get_user(fpscr, (compat_ulong_t __user *)ubuf);
 		uregs->fpsr = fpscr & VFP_FPSCR_STAT_MASK;
 		uregs->fpcr = fpscr & VFP_FPSCR_CTRL_MASK;
 	}
Signature · 7 lines
2.13.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
author
#2 lore raw

PATCH 1/1 arm64: fix missing __user in compat_vfp_{get,set}()

Quoted 3 lines · Dave Martin

(There remains a sparse warning in compat_ptrace_write_user(), where we pass implicitly cast __user onto a pointer, under set_fs(KERNEL_DS). I'm not sure what the most correct fix is there.)

Yes, I saw.
There is no pretty solution for things like here:
you can't adjust the type, you're forced to make a cast, explicit or implicit.

Regards,
-- Luc

#3 lore raw

PATCH 1/1 arm64: fix missing __user in compat_vfp_{get,set}()

In reply to Luc Van Oostenryck
Quoted 7 lines · Luc Van Oostenryck

compat_vfp_get() & compat_vfp_set() are two helpers reading or writting some values via {get,put}_user() which need a pointer annotated with '__user'.
The buffers used by the two helpers are correctly '__user' annotated but need to be casted to a real type before being given to {get,put}_user().

Quoted 8 lines

The problem is that this cast lack a '__user' annotation.

Fix this by adding the missing '__user'.

Signed-off-by Luc Van Oostenryck <[email protected]>
---
 arch/arm64/kernel/ptrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

This doesn't apply against the arm64 for-next/core branch, due to changes from Dave reworking this to use user_regset_copyout.

Will

author
#4 lore raw

PATCH v2 1/1 arm64: fix missing __user in compat_vfp_set()

compat_vfp_set() is a helper writting some values via put_user() and put_user() need a pointer annotated with '__user'. The buffer used by the helper is correctly annotated with '__user' but need to be casted to a real type before being given to put_user().

The problem is that this cast lack a '__user' annotation.

Fix this by adding the missing '__user'.

Signed-off-by Luc Van Oostenryck <[email protected]>

---
Change since v1:
- rebase against arm64/for-next/core
- drop the change for compat_vfp_get() which is no more needed

---
 arch/arm64/kernel/ptrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/ptrace.c +1 -1
index 35846f155..3a323e2b9 100644
@@ -946,7 +946,7 @@ static int compat_vfp_set(struct task_struct *target,
 				 VFP_STATE_SIZE - sizeof(compat_ulong_t));
 
 	if (count && !ret) {
-		ret = get_user(fpscr, (compat_ulong_t *)ubuf);
+		ret = get_user(fpscr, (compat_ulong_t __user *)ubuf);
 		uregs->fpsr = fpscr & VFP_FPSCR_STAT_MASK;
 		uregs->fpcr = fpscr & VFP_FPSCR_CTRL_MASK;
 	}
Signature · 1 line
2.13.0
#5 lore raw

PATCH v2 1/1 arm64: fix missing __user in compat_vfp_set()

Quoted 11 lines · Luc Van Oostenryck

compat_vfp_set() is a helper writting some values via put_user() and put_user() need a pointer annotated with '__user'. The buffer used by the helper is correctly annotated with '__user' but need to be casted to a real type before being given to put_user().

The problem is that this cast lack a '__user' annotation.

Fix this by adding the missing '__user'.

Signed-off-by Luc Van Oostenryck <[email protected]>

Apologies, I has half a reply written and then realised this issue was less straightforward that I first thought. I have an alternate patch, see [1].

Cheers
---Dave

[1] [PATCH 0/3] Miscellaneous minor compat ptrace fixes
http://lists.infradead.org/pipermail/linux-arm-kernel/2017-June/516592.html

Quoted 31 lines

---
Change since v1:
- rebase against arm64/for-next/core
- drop the change for compat_vfp_get() which is no more needed

---
 arch/arm64/kernel/ptrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/ptrace.c +1 -1
index 35846f155..3a323e2b9 100644
@@ -946,7 +946,7 @@ static int compat_vfp_set(struct task_struct *target,
 				 VFP_STATE_SIZE - sizeof(compat_ulong_t));
 
 	if (count && !ret) {
-		ret = get_user(fpscr, (compat_ulong_t *)ubuf);
+		ret = get_user(fpscr, (compat_ulong_t __user *)ubuf);
 		uregs->fpsr = fpscr & VFP_FPSCR_STAT_MASK;
 		uregs->fpcr = fpscr & VFP_FPSCR_CTRL_MASK;
 	}
Signature · 8 lines
2.13.0



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
author
#6 lore raw

PATCH v2 1/1 arm64: fix missing __user in compat_vfp_set()

Quoted 9 lines · Dave Martin
Quoted 8 lines · Luc Van Oostenryck

compat_vfp_set() is a helper writting some values via put_user() and put_user() need a pointer annotated with '__user'. The buffer used by the helper is correctly annotated with '__user' but need to be casted to a real type before being given to put_user().

The problem is that this cast lack a '__user' annotation.

Quoted 7 lines
Quoted 3 lines

Fix this by adding the missing '__user'.

Signed-off-by Luc Van Oostenryck <[email protected]>

Apologies, I has half a reply written and then realised this issue was less straightforward that I first thought. I have an alternate patch, see [1].

Ah yes.
I saw the now asymmetry between the set/copyout and the set/copyin and I was a bit surprised by it.

Your new version look better (but I can't judge more than the look).

-- Luc