* [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address
@ 2016-06-01 3:05 Yang Yingliang
2016-06-01 3:05 ` [RFC PATCH 1/4] arm64: vdso: introdce a macro for checking the address Yang Yingliang
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Yang Yingliang @ 2016-06-01 3:05 UTC (permalink / raw)
To: linux-arm-kernel
The params of gettimeofday(), clock_gettime() and clock_getres()
is not checked. This will cause segment faults when the address is
invalid.
E.g. gettimeofday(-1, -1);
clock_gettime(0, -1);
clock_getres(0, -1);
This patchset add a macro and use it to check the validation of these
pointer params.
Yang Yingliang (4):
arm64: vdso: introdce a macro for checking the address
arm64: vdso: check whether the params of gettimeofday() is valid
arm64: vdso: check whether the tp pointer is valid in clock_gettime()
arm64: vdso: check whether the res pointer is valid in clock_getres()
arch/arm64/kernel/asm-offsets.c | 3 ++
arch/arm64/kernel/vdso/gettimeofday.S | 71 +++++++++++++++++++++++++++++------
2 files changed, 62 insertions(+), 12 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/4] arm64: vdso: introdce a macro for checking the address
2016-06-01 3:05 [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address Yang Yingliang
@ 2016-06-01 3:05 ` Yang Yingliang
2016-06-01 3:06 ` [RFC PATCH 2/4] arm64: vdso: check whether the params of gettimeofday() is valid Yang Yingliang
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Yang Yingliang @ 2016-06-01 3:05 UTC (permalink / raw)
To: linux-arm-kernel
Add a RANGE_OK macro for validation checking of the
address given by user.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
arch/arm64/kernel/asm-offsets.c | 3 +++
arch/arm64/kernel/vdso/gettimeofday.S | 21 +++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 574081f..b72c2df 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -98,11 +98,14 @@ int main(void)
BLANK();
DEFINE(TVAL_TV_SEC, offsetof(struct timeval, tv_sec));
DEFINE(TVAL_TV_USEC, offsetof(struct timeval, tv_usec));
+ DEFINE(TVAL_SZ, sizeof(struct timeval));
DEFINE(TSPEC_TV_SEC, offsetof(struct timespec, tv_sec));
DEFINE(TSPEC_TV_NSEC, offsetof(struct timespec, tv_nsec));
+ DEFINE(TSPEC_SZ, sizeof(struct timespec));
BLANK();
DEFINE(TZ_MINWEST, offsetof(struct timezone, tz_minuteswest));
DEFINE(TZ_DSTTIME, offsetof(struct timezone, tz_dsttime));
+ DEFINE(TZ_SZ, sizeof(struct timezone));
BLANK();
#ifdef CONFIG_KVM_ARM_HOST
DEFINE(VCPU_CONTEXT, offsetof(struct kvm_vcpu, arch.ctxt));
diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S
index efa79e8..05ccaca 100644
--- a/arch/arm64/kernel/vdso/gettimeofday.S
+++ b/arch/arm64/kernel/vdso/gettimeofday.S
@@ -21,10 +21,31 @@
#include <linux/linkage.h>
#include <asm/asm-offsets.h>
#include <asm/unistd.h>
+#include <asm/memory.h>
#define NSEC_PER_SEC_LO16 0xca00
#define NSEC_PER_SEC_HI16 0x3b9a
+#define USER_DS TASK_SIZE_64
+/*
+ * Test whether a block of memory is a valid user space address.
+ * Returns 1 if the range is valid, 0 otherwise.
+ *
+ * This is equivalent to the following test:
+ * (u65)addr + (u65)size <= USER_DS
+ *
+ * This needs 65-bit arithmetic.
+ *
+ * x##n - addr
+ * sz - size
+ * x3 - USER_DS
+ * x4 - return value
+ */
+#define RANGE_OK(n, sz) mov x3, #USER_DS; \
+ adds x4, x##n, sz; \
+ ccmp x4, x3, #0x2, cc; \
+ cset x4, ls
+
vdso_data .req x6
use_syscall .req w7
seqcnt .req w8
--
2.5.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 2/4] arm64: vdso: check whether the params of gettimeofday() is valid
2016-06-01 3:05 [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address Yang Yingliang
2016-06-01 3:05 ` [RFC PATCH 1/4] arm64: vdso: introdce a macro for checking the address Yang Yingliang
@ 2016-06-01 3:06 ` Yang Yingliang
2016-06-01 3:06 ` [RFC PATCH 3/4] arm64: vdso: check whether the tp pointer is valid in clock_gettime() Yang Yingliang
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Yang Yingliang @ 2016-06-01 3:06 UTC (permalink / raw)
To: linux-arm-kernel
When the params of gettimeofday() is an invalid addr(E.g.
gettimeofday(-1, -1)), it will get segment fault. To avoid
this fault, use RANGE_OK to test whether a block of memory
is valid. Returns -EFAULT if the range is invalid, 0 otherwise.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
arch/arm64/kernel/vdso/gettimeofday.S | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S
index 05ccaca..43ec321 100644
--- a/arch/arm64/kernel/vdso/gettimeofday.S
+++ b/arch/arm64/kernel/vdso/gettimeofday.S
@@ -22,6 +22,7 @@
#include <asm/asm-offsets.h>
#include <asm/unistd.h>
#include <asm/memory.h>
+#include <asm-generic/errno-base.h>
#define NSEC_PER_SEC_LO16 0xca00
#define NSEC_PER_SEC_HI16 0x3b9a
@@ -78,10 +79,12 @@ ENTRY(__kernel_gettimeofday)
/* Acquire the sequence counter and get the timespec. */
adr vdso_data, _vdso_data
1: seqcnt_acquire
- cbnz use_syscall, 4f
+ cbnz use_syscall, 5f
/* If tv is NULL, skip to the timezone code. */
cbz x0, 2f
+ RANGE_OK(0, #TVAL_SZ)
+ cbz x4, 4f
bl __do_get_tspec
seqcnt_check w9, 1b
@@ -93,12 +96,18 @@ ENTRY(__kernel_gettimeofday)
2:
/* If tz is NULL, return 0. */
cbz x1, 3f
+ RANGE_OK(1, #TZ_SZ)
+ cbz x4, 4f
ldp w4, w5, [vdso_data, #VDSO_TZ_MINWEST]
stp w4, w5, [x1, #TZ_MINWEST]
3:
mov x0, xzr
ret x2
4:
+ /* tz is invalid */
+ mov x0, #-EFAULT
+ ret x2
+5:
/* Syscall fallback. */
mov x8, #__NR_gettimeofday
svc #0
--
2.5.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 3/4] arm64: vdso: check whether the tp pointer is valid in clock_gettime()
2016-06-01 3:05 [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address Yang Yingliang
2016-06-01 3:05 ` [RFC PATCH 1/4] arm64: vdso: introdce a macro for checking the address Yang Yingliang
2016-06-01 3:06 ` [RFC PATCH 2/4] arm64: vdso: check whether the params of gettimeofday() is valid Yang Yingliang
@ 2016-06-01 3:06 ` Yang Yingliang
2016-06-01 3:06 ` [RFC PATCH 4/4] arm64: vdso: check whether the res pointer is valid in clock_getres() Yang Yingliang
2016-06-06 17:12 ` [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address Will Deacon
4 siblings, 0 replies; 8+ messages in thread
From: Yang Yingliang @ 2016-06-01 3:06 UTC (permalink / raw)
To: linux-arm-kernel
Use RANGE_OK to check whether the tp pointer is valid. Returns
-EINVAL if the tp pointer is NULL, returns -EFAULT if it's invalid,
otherwise return 0.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
arch/arm64/kernel/vdso/gettimeofday.S | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S
index 43ec321..1e377ac 100644
--- a/arch/arm64/kernel/vdso/gettimeofday.S
+++ b/arch/arm64/kernel/vdso/gettimeofday.S
@@ -118,6 +118,9 @@ ENDPROC(__kernel_gettimeofday)
/* int __kernel_clock_gettime(clockid_t clock_id, struct timespec *tp); */
ENTRY(__kernel_clock_gettime)
.cfi_startproc
+ cbz x1, 7f
+ RANGE_OK(1, #TSPEC_SZ)
+ cbz x4, 8f
cmp w0, #CLOCK_REALTIME
ccmp w0, #CLOCK_MONOTONIC, #0x4, ne
b.ne 2f
@@ -128,7 +131,7 @@ ENTRY(__kernel_clock_gettime)
/* Get kernel timespec. */
adr vdso_data, _vdso_data
1: seqcnt_acquire
- cbnz use_syscall, 7f
+ cbnz use_syscall, 9f
bl __do_get_tspec
seqcnt_check w9, 1b
@@ -148,7 +151,7 @@ ENTRY(__kernel_clock_gettime)
2:
cmp w0, #CLOCK_REALTIME_COARSE
ccmp w0, #CLOCK_MONOTONIC_COARSE, #0x4, ne
- b.ne 8f
+ b.ne 10f
/* xtime_coarse_nsec is already right-shifted */
mov x12, #0
@@ -192,9 +195,15 @@ ENTRY(__kernel_clock_gettime)
stp x10, x11, [x1, #TSPEC_TV_SEC]
mov x0, xzr
ret
-7:
+7: /* tp is NULL */
+ mov x0, #-EINVAL
+ ret
+8: /* tp is invalid */
+ mov x0, #-EFAULT
+ ret
+9:
mov x30, x2
-8: /* Syscall fallback. */
+10: /* Syscall fallback. */
mov x8, #__NR_clock_gettime
svc #0
ret
--
2.5.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 4/4] arm64: vdso: check whether the res pointer is valid in clock_getres()
2016-06-01 3:05 [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address Yang Yingliang
` (2 preceding siblings ...)
2016-06-01 3:06 ` [RFC PATCH 3/4] arm64: vdso: check whether the tp pointer is valid in clock_gettime() Yang Yingliang
@ 2016-06-01 3:06 ` Yang Yingliang
2016-06-06 17:12 ` [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address Will Deacon
4 siblings, 0 replies; 8+ messages in thread
From: Yang Yingliang @ 2016-06-01 3:06 UTC (permalink / raw)
To: linux-arm-kernel
Use RANGE_OK to check whether the res pointer is valid. Returns
-EINVAL if the it's NULL, returns -EFAULT if it's invalid, otherwise
return 0.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
arch/arm64/kernel/vdso/gettimeofday.S | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S
index 1e377ac..d7e4341 100644
--- a/arch/arm64/kernel/vdso/gettimeofday.S
+++ b/arch/arm64/kernel/vdso/gettimeofday.S
@@ -217,28 +217,36 @@ ENTRY(__kernel_clock_getres)
ccmp w0, #CLOCK_MONOTONIC, #0x4, ne
b.ne 1f
- ldr x2, 5f
+ ldr x2, 6f
b 2f
1:
cmp w0, #CLOCK_REALTIME_COARSE
ccmp w0, #CLOCK_MONOTONIC_COARSE, #0x4, ne
- b.ne 4f
- ldr x2, 6f
+ b.ne 5f
+ ldr x2, 7f
2:
cbz w1, 3f
+ RANGE_OK(1, #TSPEC_SZ)
+ cbz x4, 4f
stp xzr, x2, [x1]
+ mov w0, wzr
+ ret
3: /* res == NULL. */
- mov w0, wzr
+ mov x0, #-EINVAL
ret
-4: /* Syscall fallback. */
+4: /* res is invalid. */
+ mov x0, #-EFAULT
+ ret
+
+5: /* Syscall fallback. */
mov x8, #__NR_clock_getres
svc #0
ret
-5:
- .quad CLOCK_REALTIME_RES
6:
+ .quad CLOCK_REALTIME_RES
+7:
.quad CLOCK_COARSE_RES
.cfi_endproc
ENDPROC(__kernel_clock_getres)
--
2.5.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address
2016-06-01 3:05 [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address Yang Yingliang
` (3 preceding siblings ...)
2016-06-01 3:06 ` [RFC PATCH 4/4] arm64: vdso: check whether the res pointer is valid in clock_getres() Yang Yingliang
@ 2016-06-06 17:12 ` Will Deacon
2016-06-08 9:18 ` Yang Yingliang
4 siblings, 1 reply; 8+ messages in thread
From: Will Deacon @ 2016-06-06 17:12 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 01, 2016 at 11:05:58AM +0800, Yang Yingliang wrote:
> The params of gettimeofday(), clock_gettime() and clock_getres()
> is not checked. This will cause segment faults when the address is
> invalid.
> E.g. gettimeofday(-1, -1);
> clock_gettime(0, -1);
> clock_getres(0, -1);
> This patchset add a macro and use it to check the validation of these
> pointer params.
Is this really necessary? It doesn't look to me like other architectures
bother with this, and you're just adding code to the fastpath to handle
cases that shouldn't happen to start with.
Also, I think I remember seeing SEGVs from libc wrappers for other syscalls
in the past, so this just isn't compelling to me.
Will
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address
2016-06-06 17:12 ` [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address Will Deacon
@ 2016-06-08 9:18 ` Yang Yingliang
2016-06-08 9:33 ` Will Deacon
0 siblings, 1 reply; 8+ messages in thread
From: Yang Yingliang @ 2016-06-08 9:18 UTC (permalink / raw)
To: linux-arm-kernel
On 2016/6/7 1:12, Will Deacon wrote:
> On Wed, Jun 01, 2016 at 11:05:58AM +0800, Yang Yingliang wrote:
>> The params of gettimeofday(), clock_gettime() and clock_getres()
>> is not checked. This will cause segment faults when the address is
>> invalid.
>> E.g. gettimeofday(-1, -1);
>> clock_gettime(0, -1);
>> clock_getres(0, -1);
>> This patchset add a macro and use it to check the validation of these
>> pointer params.
>
> Is this really necessary? It doesn't look to me like other architectures
> bother with this, and you're just adding code to the fastpath to handle
> cases that shouldn't happen to start with.
I agree.
I find clock_gettime() don't check if tp is NULL. I think it need a
checking like clock_getres().
What's your suggestion ?
Thanks,
Yang
>
> Also, I think I remember seeing SEGVs from libc wrappers for other syscalls
> in the past, so this just isn't compelling to me.
>
> Will
>
> .
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address
2016-06-08 9:18 ` Yang Yingliang
@ 2016-06-08 9:33 ` Will Deacon
0 siblings, 0 replies; 8+ messages in thread
From: Will Deacon @ 2016-06-08 9:33 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 08, 2016 at 05:18:26PM +0800, Yang Yingliang wrote:
>
>
> On 2016/6/7 1:12, Will Deacon wrote:
> >On Wed, Jun 01, 2016 at 11:05:58AM +0800, Yang Yingliang wrote:
> >>The params of gettimeofday(), clock_gettime() and clock_getres()
> >>is not checked. This will cause segment faults when the address is
> >>invalid.
> >>E.g. gettimeofday(-1, -1);
> >> clock_gettime(0, -1);
> >> clock_getres(0, -1);
> >>This patchset add a macro and use it to check the validation of these
> >>pointer params.
> >
> >Is this really necessary? It doesn't look to me like other architectures
> >bother with this, and you're just adding code to the fastpath to handle
> >cases that shouldn't happen to start with.
> I agree.
>
> I find clock_gettime() don't check if tp is NULL. I think it need a
> checking like clock_getres().
>
> What's your suggestion ?
So what? If you pass NULL then you get a SEGV. Just like if you pass a
NULL pointer to various glibc wrappers.
Don't pass NULL. Problem solved.
Will
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-06-08 9:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-01 3:05 [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address Yang Yingliang
2016-06-01 3:05 ` [RFC PATCH 1/4] arm64: vdso: introdce a macro for checking the address Yang Yingliang
2016-06-01 3:06 ` [RFC PATCH 2/4] arm64: vdso: check whether the params of gettimeofday() is valid Yang Yingliang
2016-06-01 3:06 ` [RFC PATCH 3/4] arm64: vdso: check whether the tp pointer is valid in clock_gettime() Yang Yingliang
2016-06-01 3:06 ` [RFC PATCH 4/4] arm64: vdso: check whether the res pointer is valid in clock_getres() Yang Yingliang
2016-06-06 17:12 ` [RFC PATCH 0/4] arm64: vdso: check whether the params is invalid address Will Deacon
2016-06-08 9:18 ` Yang Yingliang
2016-06-08 9:33 ` Will Deacon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.