linux-kernel - Re: [PATCH v3] kernel: add kcov code coverage

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 19 Jan 2016 13:47:55 +0100
From:	Dmitry Vyukov <dvyukov@...gle.com>
To:	"Kirill A. Shutemov" <kirill@...temov.name>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	David Drysdale <drysdale@...gle.com>,
	Kees Cook <keescook@...gle.com>,
	Quentin Casasnovas <quentin.casasnovas@...cle.com>,
	Sasha Levin <sasha.levin@...cle.com>,
	Vegard Nossum <vegard.nossum@...cle.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Eric Dumazet <edumazet@...gle.com>,
	Tavis Ormandy <taviso@...gle.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	syzkaller <syzkaller@...glegroups.com>,
	Kostya Serebryany <kcc@...gle.com>,
	Alexander Potapenko <glider@...gle.com>,
	Andrey Ryabinin <ryabinin.a.a@...il.com>
Subject: Re: [PATCH v3] kernel: add kcov code coverage

On Mon, Jan 18, 2016 at 10:52 PM, Kirill A. Shutemov
<kirill@...temov.name> wrote:
> On Mon, Jan 18, 2016 at 08:28:04PM +0100, Dmitry Vyukov wrote:
>> On Mon, Jan 18, 2016 at 1:42 PM, Kirill A. Shutemov
>> <kirill@...temov.name> wrote:
>> > On Thu, Jan 14, 2016 at 03:22:21PM +0100, Dmitry Vyukov wrote:
>> >> +             area = t->kcov_area;
>> >> +             /* The first u32 is number of subsequent PCs. */
>> >> +             pos = READ_ONCE(area[0]) + 1;
>> >> +             if (likely(pos < t->kcov_size)) {
>> >> +                     area[pos] = (u32)_RET_IP_;
>> >
>> > If area[0] is UINT32_MAX the condition will be true and you'll make
>> > area[0] temporary set to (u32)_RET_IP_. That's may be fun. :)
>>
>> Well, if user wants to corrupt this region, he is free to do so. But
>> it wont't corrupt kernel, right?
>
> Right. But it's easy enough to avoid overflow too:
>         "if (likely(pos && pos < t->kcov_size)"


It is easy but I don't understand what we are trying to fix.
Such check won't fix operation of kcov if user makes random memory
writes. User can also write random values which are not UINT32_MAX to
area[0]. In such case coverage will be intermixed and some pieces will
be missing.
Also this function is quite hot. A single register check and branch is
not too expensive, but it does not worth it if it does not bring any
value.


>> >> +
>> >> +     page = vmalloc_to_page(kcov->area + off);
>> >> +     get_page(page);
>> >> +     vmf->page = page;
>> >> +     return 0;
>> >> +}
>> >
>> > BTW, since all pages pre-allocated, you don't really need fault handler --
>> > just setup all pages table enties during mmap. This would shorten warm up
>> > period for userspace. See vm_insert_page().
>>
>> Done in v4
>> I don't need to get_page() before vm_insert_page(), right?
>
> Right. vm_insert_page() will take the referenece for you.


ack