* [PATCH] arm64: fix missing __user in compat_vfp_{get,set}()
@ 2017-06-28 14:58 Luc Van Oostenryck
2017-06-28 15:22 ` Dave Martin
2017-06-29 10:04 ` Will Deacon
0 siblings, 2 replies; 7+ messages in thread
From: Luc Van Oostenryck @ 2017-06-28 14:58 UTC (permalink / raw)
To: linux-arm-kernel
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 <luc.vanoostenryck@gmail.com>
---
arch/arm64/kernel/ptrace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index c142459a8..c77f425c7 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -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;
}
--
2.13.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH] arm64: fix missing __user in compat_vfp_{get,set}()
2017-06-28 14:58 [PATCH] arm64: fix missing __user in compat_vfp_{get,set}() Luc Van Oostenryck
@ 2017-06-28 15:22 ` Dave Martin
2017-06-28 16:24 ` Luc Van Oostenryck
2017-06-29 10:04 ` Will Deacon
1 sibling, 1 reply; 7+ messages in thread
From: Dave Martin @ 2017-06-28 15:22 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 28, 2017 at 04:58:16PM +0200, Luc Van Oostenryck wrote:
> 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 <Dave.Martin@arm.com>
(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
>
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
> arch/arm64/kernel/ptrace.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index c142459a8..c77f425c7 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -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;
> }
> --
> 2.13.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH] arm64: fix missing __user in compat_vfp_{get,set}()
2017-06-28 15:22 ` Dave Martin
@ 2017-06-28 16:24 ` Luc Van Oostenryck
0 siblings, 0 replies; 7+ messages in thread
From: Luc Van Oostenryck @ 2017-06-28 16:24 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 28, 2017 at 04:22:22PM +0100, Dave Martin wrote:
> (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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] arm64: fix missing __user in compat_vfp_{get,set}()
2017-06-28 14:58 [PATCH] arm64: fix missing __user in compat_vfp_{get,set}() Luc Van Oostenryck
2017-06-28 15:22 ` Dave Martin
@ 2017-06-29 10:04 ` Will Deacon
2017-06-29 14:24 ` [PATCH v2] arm64: fix missing __user in compat_vfp_set() Luc Van Oostenryck
1 sibling, 1 reply; 7+ messages in thread
From: Will Deacon @ 2017-06-29 10:04 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 28, 2017 at 04:58:16PM +0200, Luc Van Oostenryck wrote:
> 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 <luc.vanoostenryck@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2] arm64: fix missing __user in compat_vfp_set()
2017-06-29 10:04 ` Will Deacon
@ 2017-06-29 14:24 ` Luc Van Oostenryck
2017-06-29 14:34 ` Dave Martin
0 siblings, 1 reply; 7+ messages in thread
From: Luc Van Oostenryck @ 2017-06-29 14:24 UTC (permalink / raw)
To: linux-arm-kernel
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 <luc.vanoostenryck@gmail.com>
---
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(-)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 35846f155..3a323e2b9 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -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;
}
--
2.13.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2] arm64: fix missing __user in compat_vfp_set()
2017-06-29 14:24 ` [PATCH v2] arm64: fix missing __user in compat_vfp_set() Luc Van Oostenryck
@ 2017-06-29 14:34 ` Dave Martin
2017-06-29 16:23 ` Luc Van Oostenryck
0 siblings, 1 reply; 7+ messages in thread
From: Dave Martin @ 2017-06-29 14:34 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 29, 2017 at 04:24:48PM +0200, Luc Van Oostenryck wrote:
> 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 <luc.vanoostenryck@gmail.com>
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
>
> ---
> 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(-)
>
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 35846f155..3a323e2b9 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -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;
> }
> --
> 2.13.0
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2] arm64: fix missing __user in compat_vfp_set()
2017-06-29 14:34 ` Dave Martin
@ 2017-06-29 16:23 ` Luc Van Oostenryck
0 siblings, 0 replies; 7+ messages in thread
From: Luc Van Oostenryck @ 2017-06-29 16:23 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 29, 2017 at 03:34:06PM +0100, Dave Martin wrote:
> On Thu, Jun 29, 2017 at 04:24:48PM +0200, Luc Van Oostenryck wrote:
> > 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 <luc.vanoostenryck@gmail.com>
>
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-06-29 16:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-28 14:58 [PATCH] arm64: fix missing __user in compat_vfp_{get,set}() Luc Van Oostenryck
2017-06-28 15:22 ` Dave Martin
2017-06-28 16:24 ` Luc Van Oostenryck
2017-06-29 10:04 ` Will Deacon
2017-06-29 14:24 ` [PATCH v2] arm64: fix missing __user in compat_vfp_set() Luc Van Oostenryck
2017-06-29 14:34 ` Dave Martin
2017-06-29 16:23 ` Luc Van Oostenryck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).