[PATCH] time: do a safe overflow check in ktime_add_safe

[PATCH] time: do a safe overflow check in ktime_add_safe

[Sasha Levin]

ktime_add_safe would check for overflows, but since ktime variables are
signed, overflowing them is an undefined behaviour and should be avoided.

Rather than checking for wraparound after the overflow, check for
potential overflowing values prior to adding both ktimes.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 kernel/time/hrtimer.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 37e50aa..42fb631 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -290,16 +290,14 @@ EXPORT_SYMBOL_GPL(ktime_divns);
  */
 ktime_t ktime_add_safe(const ktime_t lhs, const ktime_t rhs)
 {
-	ktime_t res = ktime_add(lhs, rhs);
-
 	/*
 	 * We use KTIME_SEC_MAX here, the maximum timeout which we can
 	 * return to user space in a timespec:
 	 */
-	if (res.tv64 < 0 || res.tv64 < lhs.tv64 || res.tv64 < rhs.tv64)
-		res = ktime_set(KTIME_SEC_MAX, 0);
+	if (lhs.tv64 > (KTIME_MAX - rhs.tv64))
+		return ktime_set(KTIME_SEC_MAX, 0);
 
-	return res;
+	return ktime_add(lhs, rhs);
 }
 
 EXPORT_SYMBOL_GPL(ktime_add_safe);
-- 
1.7.10.4

[PATCH] time: make sure tz_minuteswest is set to a valid value when setting time

[Sasha Levin]

Invalid values may overflow later, leading to undefined behaviour when
multiplied by 60 to get the amount of seconds.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 kernel/time/time.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/time/time.c b/kernel/time/time.c
index 7399a73..9ec4fa5 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -173,6 +173,10 @@ int do_sys_settimeofday(const struct timespec *tv, const struct timezone *tz)
 		return error;
 
 	if (tz) {
+		/* Verify we're witin the +-15 hrs range */
+		if (tz->tz_minuteswest > 15*60 || tz->tz_minuteswest < -15*60)
+			return -EINVAL;
+
 		sys_tz = *tz;
 		update_vsyscall_tz();
 		if (firsttime) {
-- 
1.7.10.4

[PATCH] vfs: calculate seek offsets using unsigned variables

[Sasha Levin]

Adding two loff_ts means adding two signed integers. Adding two such integers
when they are unchecked might cause an overflow which is undefined for
signed integers.

Avoid it by doing the math using unsigned cast and casting it back implicitly
into loff_t.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 fs/read_write.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index c0805c9..54311f4 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -91,7 +91,7 @@ generic_file_llseek_size(struct file *file, loff_t offset, int whence,
 {
 	switch (whence) {
 	case SEEK_END:
-		offset += eof;
+		offset += (u64)eof;
 		break;
 	case SEEK_CUR:
 		/*
@@ -108,7 +108,7 @@ generic_file_llseek_size(struct file *file, loff_t offset, int whence,
 		 * like SEEK_SET.
 		 */
 		spin_lock(&file->f_lock);
-		offset = vfs_setpos(file, file->f_pos + offset, maxsize);
+		offset = vfs_setpos(file, (u64)file->f_pos + offset, maxsize);
 		spin_unlock(&file->f_lock);
 		return offset;
 	case SEEK_DATA:
-- 
1.7.10.4

[PATCH] mm: fadvise: avoid signed integer overflow calculating offset

[Sasha Levin]

Both offset and len are signed integers who's overflow isn't defined. Use
unsigned addition to avoid the issue.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 mm/fadvise.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/fadvise.c b/mm/fadvise.c
index 3bcfd81d..762cb63 100644
--- a/mm/fadvise.c
+++ b/mm/fadvise.c
@@ -67,7 +67,7 @@ SYSCALL_DEFINE4(fadvise64_64, int, fd, loff_t, offset, loff_t, len, int, advice)
 	}
 
 	/* Careful about overflows. Len == 0 means "as much as possible" */
-	endbyte = offset + len;
+	endbyte = offset + (u64)len;
 	if (!len || endbyte < len)
 		endbyte = -1;
 	else
-- 
1.7.10.4

[PATCH] time: settimeofday: validate the values of tv fomr user

[Sasha Levin]

An unvalidated user input is multiplied by a constant, which can result in
an undefined behaviour for large values. While this is validated later,
we should avoid triggering undefined behaviour.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 kernel/time/time.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/time/time.c b/kernel/time/time.c
index 9ec4fa5..6f53df7 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -200,6 +200,10 @@ SYSCALL_DEFINE2(settimeofday, struct timeval __user *, tv,
 	if (tv) {
 		if (copy_from_user(&user_tv, tv, sizeof(*tv)))
 			return -EFAULT;
+
+		if (user_tv.tv_usec > USEC_PER_SEC || user_tv.tv_usec < 0)
+			return -EINVAL;
+
 		new_ts.tv_sec = user_tv.tv_sec;
 		new_ts.tv_nsec = user_tv.tv_usec * NSEC_PER_USEC;
 	}
-- 
1.7.10.4

[PATCH] fs: sync_file_range: avoid overflowing signed calculation

[Sasha Levin]

When calculating the end byte for the sync we preform an addition which might
result in an overflow, which is undefined. Avoid it by doing unsigned
addition.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 fs/sync.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/sync.c b/fs/sync.c
index 6ccfc38..f9cb38f 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -286,7 +286,7 @@ SYSCALL_DEFINE4(sync_file_range, int, fd, loff_t, offset, loff_t, nbytes,
 	if (flags & ~VALID_FLAGS)
 		goto out;
 
-	endbyte = offset + nbytes;
+	endbyte = offset + (u64)nbytes;
 
 	if ((s64)offset < 0)
 		goto out;
-- 
1.7.10.4

Re: [PATCH] time: settimeofday: validate the values of tv fomr user

[Thomas Gleixner]

On Mon, 1 Dec 2014, Sasha Levin wrote:

> An unvalidated user input is multiplied by a constant, which can result in
> an undefined behaviour for large values. While this is validated later,
> we should avoid triggering undefined behaviour.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  kernel/time/time.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/time/time.c b/kernel/time/time.c
> index 9ec4fa5..6f53df7 100644
> --- a/kernel/time/time.c
> +++ b/kernel/time/time.c
> @@ -200,6 +200,10 @@ SYSCALL_DEFINE2(settimeofday, struct timeval __user *, tv,
>  	if (tv) {
>  		if (copy_from_user(&user_tv, tv, sizeof(*tv)))
>  			return -EFAULT;
> +
> +		if (user_tv.tv_usec > USEC_PER_SEC || user_tv.tv_usec < 0)
> +			return -EINVAL;

We should create timeval_valid() for this with the same logic as
timespec_valid().

Thanks,

	tglx