linux-kernel - Re: int overflow in io

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 16 Dec 2015 19:38:33 +0100
From:	Dmitry Vyukov <dvyukov@...gle.com>
To:	Jan Kara <jack@...e.cz>
Cc:	Benjamin LaHaise <bcrl@...ck.org>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-aio <linux-aio@...ck.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	syzkaller <syzkaller@...glegroups.com>,
	Kostya Serebryany <kcc@...gle.com>,
	Alexander Potapenko <glider@...gle.com>,
	Sasha Levin <sasha.levin@...cle.com>,
	Andrey Ryabinin <ryabinin.a.a@...il.com>
Subject: Re: int overflow in io_getevents

On Wed, Dec 16, 2015 at 1:56 PM, Jan Kara <jack@...e.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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/