int overflow in io_getevents

int overflow in io_getevents

[Dmitry Vyukov]

Hello,

While running syzkaller fuzzer on commit
31ade3b83e1821da5fbb2f11b5b3d4ab2ec39db8, I've hit the following UBSAN
warning. I think it can lead to an unexpected active wait loop, if
user-space expects such io_getevents to wait for a long duration but
instead it returns immediately, so user-space reissues the same call
again and again. Andrey suggested that read_events should validate
timeout with timespec_valid_strict before using it.


UBSAN: Undefined behaviour in include/linux/ktime.h:55:49
signed integer overflow:
1449363382000000000 + 8584381026499825158 cannot be represented in
type 'long long int'
CPU: 0 PID: 27992 Comm: syzkaller_execu Not tainted 4.4.0-rc3+ #150
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 0000000000000000 ffff880062ab7ae0 ffffffff82c6f2a8 0000000041b58ab3
 ffffffff8788bf8d ffffffff82c6f1f6 ffff880062ab7aa8 ffffffff88479680
 ffff880062ab7be0 7721d90fc5200e06 0000000000000001 ffff880062ab7af0
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff82c6f2a8>] dump_stack+0xb2/0xfa lib/dump_stack.c:50
 [<ffffffff82d622e7>] ubsan_epilogue+0x12/0x8f lib/ubsan.c:160
 [<ffffffff82d63e58>] handle_overflow+0x22f/0x276 lib/ubsan.c:191
 [<ffffffff82d63ec9>] __ubsan_handle_add_overflow+0x2a/0x31 lib/ubsan.c:199
 [<     inline     >] ktime_set include/linux/ktime.h:55
 [<     inline     >] timespec_to_ktime include/linux/ktime.h:83
 [<ffffffff8190ca77>] read_events+0x4b7/0x560 fs/aio.c:1273
 [<     inline     >] SYSC_io_getevents fs/aio.c:1737
 [<ffffffff81913567>] SyS_io_getevents+0xc7/0x340 fs/aio.c:1726
 [<ffffffff868dae76>] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185

Thank you

Re: int overflow in io_getevents

[Jan Kara]

On Mon 07-12-15 11:27:07, Dmitry Vyukov wrote:
> Hello,
> 
> While running syzkaller fuzzer on commit
> 31ade3b83e1821da5fbb2f11b5b3d4ab2ec39db8, I've hit the following UBSAN
> warning. I think it can lead to an unexpected active wait loop, if
> user-space expects such io_getevents to wait for a long duration but
> instead it returns immediately, so user-space reissues the same call
> again and again. Andrey suggested that read_events should validate
> timeout with timespec_valid_strict before using it.

Yup, looks correct. Will you send a patch?

								Honza

> UBSAN: Undefined behaviour in include/linux/ktime.h:55:49
> signed integer overflow:
> 1449363382000000000 + 8584381026499825158 cannot be represented in
> type 'long long int'
> CPU: 0 PID: 27992 Comm: syzkaller_execu Not tainted 4.4.0-rc3+ #150
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>  0000000000000000 ffff880062ab7ae0 ffffffff82c6f2a8 0000000041b58ab3
>  ffffffff8788bf8d ffffffff82c6f1f6 ffff880062ab7aa8 ffffffff88479680
>  ffff880062ab7be0 7721d90fc5200e06 0000000000000001 ffff880062ab7af0
> Call Trace:
>  [<     inline     >] __dump_stack lib/dump_stack.c:15
>  [<ffffffff82c6f2a8>] dump_stack+0xb2/0xfa lib/dump_stack.c:50
>  [<ffffffff82d622e7>] ubsan_epilogue+0x12/0x8f lib/ubsan.c:160
>  [<ffffffff82d63e58>] handle_overflow+0x22f/0x276 lib/ubsan.c:191
>  [<ffffffff82d63ec9>] __ubsan_handle_add_overflow+0x2a/0x31 lib/ubsan.c:199
>  [<     inline     >] ktime_set include/linux/ktime.h:55
>  [<     inline     >] timespec_to_ktime include/linux/ktime.h:83
>  [<ffffffff8190ca77>] read_events+0x4b7/0x560 fs/aio.c:1273
>  [<     inline     >] SYSC_io_getevents fs/aio.c:1737
>  [<ffffffff81913567>] SyS_io_getevents+0xc7/0x340 fs/aio.c:1726
>  [<ffffffff868dae76>] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
> 
> Thank you
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: int overflow in io_getevents

[Dmitry Vyukov]

On Wed, Dec 16, 2015 at 1:56 PM, Jan Kara <jack@suse.cz> wrote:
> On Mon 07-12-15 11:27:07, Dmitry Vyukov wrote:
>> Hello,
>>
>> While running syzkaller fuzzer on commit
>> 31ade3b83e1821da5fbb2f11b5b3d4ab2ec39db8, I've hit the following UBSAN
>> warning. I think it can lead to an unexpected active wait loop, if
>> user-space expects such io_getevents to wait for a long duration but
>> instead it returns immediately, so user-space reissues the same call
>> again and again. Andrey suggested that read_events should validate
>> timeout with timespec_valid_strict before using it.
>
> Yup, looks correct. Will you send a patch?

I've drafted the verification:

@@ -1269,6 +1269,8 @@ static long read_events(struct kioctx *ctx, long
min_nr, long nr,

                if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
                        return -EFAULT;
+               if (!timespec_valid_strict(&strict))
+                       return -EINVAL;

                until = timespec_to_ktime(ts);
        }

But now I am thinking whether it is the right solution.
First, user does not know about KTIME_MAX, so it is not unreasonable
to pass timespec{INT64_MAX, INT64_MAX} as timeout expecting that it
will block for a long time. And it actually probably mostly works now,
because after the overflow you still get something large with high
probability. If we do the fix, then users will need to pass seconds <
KTIME_MAX, while they don't know KTIME_MAX value.
Second, there seems to be more serious issue in ktime_set() which
checks seconds for KTIME_MAX, but on the next line addition still
overflows int64.
Thoughts?

Re: int overflow in io_getevents

[Jan Kara]

On Wed 16-12-15 19:38:33, Dmitry Vyukov wrote:
> On Wed, Dec 16, 2015 at 1:56 PM, Jan Kara <jack@suse.cz> wrote:
> > On Mon 07-12-15 11:27:07, Dmitry Vyukov wrote:
> >> Hello,
> >>
> >> While running syzkaller fuzzer on commit
> >> 31ade3b83e1821da5fbb2f11b5b3d4ab2ec39db8, I've hit the following UBSAN
> >> warning. I think it can lead to an unexpected active wait loop, if
> >> user-space expects such io_getevents to wait for a long duration but
> >> instead it returns immediately, so user-space reissues the same call
> >> again and again. Andrey suggested that read_events should validate
> >> timeout with timespec_valid_strict before using it.
> >
> > Yup, looks correct. Will you send a patch?
> 
> I've drafted the verification:
> 
> @@ -1269,6 +1269,8 @@ static long read_events(struct kioctx *ctx, long
> min_nr, long nr,
> 
>                 if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
>                         return -EFAULT;
> +               if (!timespec_valid_strict(&strict))
> +                       return -EINVAL;
> 
>                 until = timespec_to_ktime(ts);
>         }
> 
> But now I am thinking whether it is the right solution.
> First, user does not know about KTIME_MAX, so it is not unreasonable
> to pass timespec{INT64_MAX, INT64_MAX} as timeout expecting that it
> will block for a long time. And it actually probably mostly works now,
> because after the overflow you still get something large with high
> probability. If we do the fix, then users will need to pass seconds <
> KTIME_MAX, while they don't know KTIME_MAX value.
> Second, there seems to be more serious issue in ktime_set() which
> checks seconds for KTIME_MAX, but on the next line addition still
> overflows int64.

Frankly, if you don't want the timeout (and overflowing ktime effectively
means you don't want it), you shouldn't set timeout at all. So I'd be in
favor of the check and EINVAL return value. If we find out some userspace
is broken (and indeed I can imagine someone accidentally passes e.g.
uninitialized 'timeout' and it happens to work), we could always trim too
big timeout to KTIME_MAX. But first I'd try the strict check and see what
breaks ;).

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: int overflow in io_getevents

[Benjamin LaHaise]

On Wed, Dec 16, 2015 at 07:38:33PM +0100, Dmitry Vyukov wrote:
> > Yup, looks correct. Will you send a patch?
> 
> I've drafted the verification:
> 
> @@ -1269,6 +1269,8 @@ static long read_events(struct kioctx *ctx, long
> min_nr, long nr,
> 
>                 if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
>                         return -EFAULT;
> +               if (!timespec_valid_strict(&strict))
> +                       return -EINVAL;
> 
>                 until = timespec_to_ktime(ts);
>         }
> 
> But now I am thinking whether it is the right solution.
> First, user does not know about KTIME_MAX, so it is not unreasonable
> to pass timespec{INT64_MAX, INT64_MAX} as timeout expecting that it
> will block for a long time. And it actually probably mostly works now,
> because after the overflow you still get something large with high
> probability. If we do the fix, then users will need to pass seconds <
> KTIME_MAX, while they don't know KTIME_MAX value.
> Second, there seems to be more serious issue in ktime_set() which
> checks seconds for KTIME_MAX, but on the next line addition still
> overflows int64.
> Thoughts?

I finally had some time to look over this after the holidays, and I 
don't think using timespec_valid_strict() is the right approach here, 
as userspace will have no idea what KTIME_MAX is.  Instead, I think the 
right approach is to -EINVAL for negative values (which should avoid 
the overflow), and to allow too large values to be silently truncated 
by timespec_to_ktime().  The truncation doesn't matter all that much 
given that it's in the hundreds of years ballpark.  I'll push the patch 
below if there are no objections.

		-ben
-- 
"Thought is the essence of where you are now."

commit 4304367826d0df42086ef24428c6c277acd822a9
Author: Benjamin LaHaise <bcrl@kvack.org>
Date:   Wed Jan 6 12:46:12 2016 -0500

    aio: handle integer overflow in io_getevents() timespec usage
    
    Dmitry Vyukov reported an integer overflow in io_getevents() when
    running a fuzzer.  Upon investigation, the triggers appears to be that a
    negative value for the tv_sec or tv_nsec was passed in which is not
    handled by timespec_to_ktime().  This patch fixes that by making
    io_getevents() return -EINVAL when negative timeouts are passed in.
    
    Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>

diff --git a/fs/aio.c b/fs/aio.c
index 155f842..f325ed4 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1269,6 +1269,8 @@ static long read_events(struct kioctx *ctx, long min_nr, long nr,
 
 		if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
 			return -EFAULT;
+		if ((ts.tv_sec < 0) || (ts.tv_nsec < 0))
+			return -EINVAL;
 
 		until = timespec_to_ktime(ts);
 	}

Re: int overflow in io_getevents

[Dmitry Vyukov]

On Wed, Jan 6, 2016 at 7:01 PM, Benjamin LaHaise <bcrl@kvack.org> wrote:
> On Wed, Dec 16, 2015 at 07:38:33PM +0100, Dmitry Vyukov wrote:
>> > Yup, looks correct. Will you send a patch?
>>
>> I've drafted the verification:
>>
>> @@ -1269,6 +1269,8 @@ static long read_events(struct kioctx *ctx, long
>> min_nr, long nr,
>>
>>                 if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
>>                         return -EFAULT;
>> +               if (!timespec_valid_strict(&strict))
>> +                       return -EINVAL;
>>
>>                 until = timespec_to_ktime(ts);
>>         }
>>
>> But now I am thinking whether it is the right solution.
>> First, user does not know about KTIME_MAX, so it is not unreasonable
>> to pass timespec{INT64_MAX, INT64_MAX} as timeout expecting that it
>> will block for a long time. And it actually probably mostly works now,
>> because after the overflow you still get something large with high
>> probability. If we do the fix, then users will need to pass seconds <
>> KTIME_MAX, while they don't know KTIME_MAX value.
>> Second, there seems to be more serious issue in ktime_set() which
>> checks seconds for KTIME_MAX, but on the next line addition still
>> overflows int64.
>> Thoughts?
>
> I finally had some time to look over this after the holidays, and I
> don't think using timespec_valid_strict() is the right approach here,
> as userspace will have no idea what KTIME_MAX is.  Instead, I think the
> right approach is to -EINVAL for negative values (which should avoid
> the overflow), and to allow too large values to be silently truncated
> by timespec_to_ktime().  The truncation doesn't matter all that much
> given that it's in the hundreds of years ballpark.  I'll push the patch
> below if there are no objections.
>
>                 -ben
> --
> "Thought is the essence of where you are now."
>
> commit 4304367826d0df42086ef24428c6c277acd822a9
> Author: Benjamin LaHaise <bcrl@kvack.org>
> Date:   Wed Jan 6 12:46:12 2016 -0500
>
>     aio: handle integer overflow in io_getevents() timespec usage
>
>     Dmitry Vyukov reported an integer overflow in io_getevents() when
>     running a fuzzer.  Upon investigation, the triggers appears to be that a
>     negative value for the tv_sec or tv_nsec was passed in which is not
>     handled by timespec_to_ktime().  This patch fixes that by making
>     io_getevents() return -EINVAL when negative timeouts are passed in.
>
>     Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 155f842..f325ed4 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1269,6 +1269,8 @@ static long read_events(struct kioctx *ctx, long min_nr, long nr,
>
>                 if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
>                         return -EFAULT;
> +               if ((ts.tv_sec < 0) || (ts.tv_nsec < 0))
> +                       return -EINVAL;
>
>                 until = timespec_to_ktime(ts);
>         }


Sorry, but the following program still prints -9223372036562067969. I
think timespec_valid check will do.

#include <stdio.h>
#include <limits.h>

typedef long s64;
typedef unsigned long u64;

#define TIME64_MAX                      ((s64)~((u64)1 << 63))
#define KTIME_MAX                       ((s64)~((u64)1 << 63))
#define KTIME_SEC_MAX                   (KTIME_MAX / NSEC_PER_SEC)
#define NSEC_PER_SEC    1000000000L
#define unlikely(x) x

struct timespec {
        long            tv_sec;                 /* seconds */
        long            tv_nsec;               /* nanoseconds */
};

union ktime {
        s64     tv64;
};

typedef union ktime ktime_t;

static inline ktime_t ktime_set(const s64 secs, const unsigned long nsecs)
{
        if (unlikely(secs >= KTIME_SEC_MAX))
                return (ktime_t){ .tv64 = KTIME_MAX };

        return (ktime_t) { .tv64 = secs * NSEC_PER_SEC + (s64)nsecs };
}

static inline ktime_t timespec_to_ktime(struct timespec ts)
{
        return ktime_set(ts.tv_sec, ts.tv_nsec);
}

int main(void)
{
        struct timespec ts = {KTIME_SEC_MAX - 1, INT_MAX};
        ktime_t t;

        if ((ts.tv_sec < 0) || (ts.tv_nsec < 0))
                return 0;
        t = timespec_to_ktime(ts);
        printf("%ld\n", t.tv64);
        return 0;
}

Re: int overflow in io_getevents

[Benjamin LaHaise]

On Thu, Jan 07, 2016 at 10:12:02AM +0100, Dmitry Vyukov wrote:
...
> Sorry, but the following program still prints -9223372036562067969. I
> think timespec_valid check will do.

Ah, right.  Yes, using timespec_valid() instead of  timespec_valid_strict() 
as initially proposed will address my concerns.  Updated commit below.

		-ben
-- 
"Thought is the essence of where you are now."

commit 3a55c535cf3836257434518bd6bc11464c493492
Author: Benjamin LaHaise <bcrl@kvack.org>
Date:   Thu Jan 7 10:25:44 2016 -0500

    aio: handle integer overflow in io_getevents() timespec usage
    
    Dmitry Vyukov reported an integer overflow in io_getevents() when
    running a fuzzer.  Upon investigation, the triggers appears to be that
    an invalid value for the tv_sec or tv_nsec was passed in which is not
    handled by timespec_to_ktime().  This patch fixes that by making
    io_getevents() return -EINVAL when timespec_valid() checks fail.  We
    use timespec_valid() instead of timespec_valid_strict() to avoid issues
    caused by userspace not knowing the cutoff for KTIME_SEC_MAX.
    
    Reported-by: Dmitry Vyukov <dvyukov@google.com>
    Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>

diff --git a/fs/aio.c b/fs/aio.c
index 155f842..d161a2f 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1269,6 +1269,8 @@ static long read_events(struct kioctx *ctx, long min_nr, long nr,
 
 		if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
 			return -EFAULT;
+		if (!timespec_valid())
+			return -EINVAL;
 
 		until = timespec_to_ktime(ts);
 	}

Re: int overflow in io_getevents

[Dmitry Vyukov]

On Thu, Jan 7, 2016 at 4:31 PM, Benjamin LaHaise <bcrl@kvack.org> wrote:
> On Thu, Jan 07, 2016 at 10:12:02AM +0100, Dmitry Vyukov wrote:
> ...
>> Sorry, but the following program still prints -9223372036562067969. I
>> think timespec_valid check will do.
>
> Ah, right.  Yes, using timespec_valid() instead of  timespec_valid_strict()
> as initially proposed will address my concerns.  Updated commit below.
>
>                 -ben
> --
> "Thought is the essence of where you are now."
>
> commit 3a55c535cf3836257434518bd6bc11464c493492
> Author: Benjamin LaHaise <bcrl@kvack.org>
> Date:   Thu Jan 7 10:25:44 2016 -0500
>
>     aio: handle integer overflow in io_getevents() timespec usage
>
>     Dmitry Vyukov reported an integer overflow in io_getevents() when
>     running a fuzzer.  Upon investigation, the triggers appears to be that
>     an invalid value for the tv_sec or tv_nsec was passed in which is not
>     handled by timespec_to_ktime().  This patch fixes that by making
>     io_getevents() return -EINVAL when timespec_valid() checks fail.  We
>     use timespec_valid() instead of timespec_valid_strict() to avoid issues
>     caused by userspace not knowing the cutoff for KTIME_SEC_MAX.
>
>     Reported-by: Dmitry Vyukov <dvyukov@google.com>
>     Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 155f842..d161a2f 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1269,6 +1269,8 @@ static long read_events(struct kioctx *ctx, long min_nr, long nr,
>
>                 if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
>                         return -EFAULT;
> +               if (!timespec_valid())

pass ts to the function

> +                       return -EINVAL;
>
>                 until = timespec_to_ktime(ts);
>         }

Re: int overflow in io_getevents

[Benjamin LaHaise]

On Thu, Jan 07, 2016 at 04:37:43PM +0100, Dmitry Vyukov wrote:
> pass ts to the function

Yeah, I should have had my morning coffee before hitting send.  Updated 
below, and hopefully final.  Checked with a test program to confirm that 
the huge value of seconds in timespec correctly waits, and that negative 
or other invalid values fail with EINVAL (download from
http://www.kvack.org/~bcrl/aio-io_getevents-timespec.c ).

		-ben
-- 
"Thought is the essence of where you are now."

commit 49b78150bc5762c58cfb8b19a859c354cf1a71ac
Author: Benjamin LaHaise <bcrl@kvack.org>
Date:   Thu Jan 7 10:37:58 2016 -0500

    aio: handle integer overflow in io_getevents() timespec usage
    
    Dmitry Vyukov reported an integer overflow in io_getevents() when
    running a fuzzer.  Upon investigation, the triggers appears to be that
    an invalid value for the tv_sec or tv_nsec was passed in which is not
    handled by timespec_to_ktime().  This patch fixes that by making
    io_getevents() return -EINVAL when timespec_valid() checks fail.  We
    use timespec_valid() instead of timespec_valid_strict() to avoid issues
    caused by userspace not knowing the cutoff for KTIME_SEC_MAX.
    
    Reported-by: Dmitry Vyukov <dvyukov@google.com>
    Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>

diff --git a/fs/aio.c b/fs/aio.c
index 155f842..e0d5398 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1269,6 +1269,8 @@ static long read_events(struct kioctx *ctx, long min_nr, long nr,
 
 		if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
 			return -EFAULT;
+		if (!timespec_valid(&ts))
+			return -EINVAL;
 
 		until = timespec_to_ktime(ts);
 	}

Re: int overflow in io_getevents

[Dmitry Vyukov]

On Thu, Jan 7, 2016 at 4:52 PM, Benjamin LaHaise <bcrl@kvack.org> wrote:
> On Thu, Jan 07, 2016 at 04:37:43PM +0100, Dmitry Vyukov wrote:
>> pass ts to the function
>
> Yeah, I should have had my morning coffee before hitting send.  Updated
> below, and hopefully final.  Checked with a test program to confirm that
> the huge value of seconds in timespec correctly waits, and that negative
> or other invalid values fail with EINVAL (download from
> http://www.kvack.org/~bcrl/aio-io_getevents-timespec.c ).
>
>                 -ben
> --
> "Thought is the essence of where you are now."


Acked-by: Dmitry Vyukov <dvyukov@google.com>

Thanks!

> commit 49b78150bc5762c58cfb8b19a859c354cf1a71ac
> Author: Benjamin LaHaise <bcrl@kvack.org>
> Date:   Thu Jan 7 10:37:58 2016 -0500
>
>     aio: handle integer overflow in io_getevents() timespec usage
>
>     Dmitry Vyukov reported an integer overflow in io_getevents() when
>     running a fuzzer.  Upon investigation, the triggers appears to be that
>     an invalid value for the tv_sec or tv_nsec was passed in which is not
>     handled by timespec_to_ktime().  This patch fixes that by making
>     io_getevents() return -EINVAL when timespec_valid() checks fail.  We
>     use timespec_valid() instead of timespec_valid_strict() to avoid issues
>     caused by userspace not knowing the cutoff for KTIME_SEC_MAX.
>
>     Reported-by: Dmitry Vyukov <dvyukov@google.com>
>     Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 155f842..e0d5398 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1269,6 +1269,8 @@ static long read_events(struct kioctx *ctx, long min_nr, long nr,
>
>                 if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
>                         return -EFAULT;
> +               if (!timespec_valid(&ts))
> +                       return -EINVAL;
>
>                 until = timespec_to_ktime(ts);
>         }

Re: int overflow in io_getevents

[Jiri Slaby]

Hi,

what's the status of this? I have just hit it now and don't see it merged.

On 01/07/2016, 05:27 PM, Dmitry Vyukov wrote:
> On Thu, Jan 7, 2016 at 4:52 PM, Benjamin LaHaise <bcrl@kvack.org> wrote:
>> On Thu, Jan 07, 2016 at 04:37:43PM +0100, Dmitry Vyukov wrote:
>>> pass ts to the function
>>
>> Yeah, I should have had my morning coffee before hitting send.  Updated
>> below, and hopefully final.  Checked with a test program to confirm that
>> the huge value of seconds in timespec correctly waits, and that negative
>> or other invalid values fail with EINVAL (download from
>> http://www.kvack.org/~bcrl/aio-io_getevents-timespec.c ).
>>
>>                 -ben
>> --
>> "Thought is the essence of where you are now."
> 
> 
> Acked-by: Dmitry Vyukov <dvyukov@google.com>
> 
> Thanks!
> 
>> commit 49b78150bc5762c58cfb8b19a859c354cf1a71ac
>> Author: Benjamin LaHaise <bcrl@kvack.org>
>> Date:   Thu Jan 7 10:37:58 2016 -0500
>>
>>     aio: handle integer overflow in io_getevents() timespec usage
>>
>>     Dmitry Vyukov reported an integer overflow in io_getevents() when
>>     running a fuzzer.  Upon investigation, the triggers appears to be that
>>     an invalid value for the tv_sec or tv_nsec was passed in which is not
>>     handled by timespec_to_ktime().  This patch fixes that by making
>>     io_getevents() return -EINVAL when timespec_valid() checks fail.  We
>>     use timespec_valid() instead of timespec_valid_strict() to avoid issues
>>     caused by userspace not knowing the cutoff for KTIME_SEC_MAX.
>>
>>     Reported-by: Dmitry Vyukov <dvyukov@google.com>
>>     Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
>>
>> diff --git a/fs/aio.c b/fs/aio.c
>> index 155f842..e0d5398 100644
>> --- a/fs/aio.c
>> +++ b/fs/aio.c
>> @@ -1269,6 +1269,8 @@ static long read_events(struct kioctx *ctx, long min_nr, long nr,
>>
>>                 if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
>>                         return -EFAULT;
>> +               if (!timespec_valid(&ts))
>> +                       return -EINVAL;
>>
>>                 until = timespec_to_ktime(ts);


-- 
js
suse labs