From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1756687AbcARVwH (ORCPT <rfc822;w@1wt.eu>);
	Mon, 18 Jan 2016 16:52:07 -0500
Received: from mail-wm0-f68.google.com ([74.125.82.68]:36856 "EHLO
	mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1756558AbcARVwF (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 18 Jan 2016 16:52:05 -0500
Date: Mon, 18 Jan 2016 23:52:01 +0200
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
        David Drysdale <drysdale@google.com>, Kees Cook <keescook@google.com>,
        Quentin Casasnovas <quentin.casasnovas@oracle.com>,
        Sasha Levin <sasha.levin@oracle.com>,
        Vegard Nossum <vegard.nossum@oracle.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Eric Dumazet <edumazet@google.com>, Tavis Ormandy <taviso@google.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        syzkaller <syzkaller@googlegroups.com>,
        Kostya Serebryany <kcc@google.com>,
        Alexander Potapenko <glider@google.com>,
        Andrey Ryabinin <ryabinin.a.a@gmail.com>
Subject: Re: [PATCH v3] kernel: add kcov code coverage
Message-ID: <20160118215201.GD14531@node.shutemov.name>
References: <1452781341-34178-1-git-send-email-dvyukov@google.com>
 <20160118124201.GB14531@node.shutemov.name>
 <CACT4Y+b72eard_VXaFLa9xh-OZWj7Y5LHsoFP0jvxVyLVxjoZQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CACT4Y+b72eard_VXaFLa9xh-OZWj7Y5LHsoFP0jvxVyLVxjoZQ@mail.gmail.com>
User-Agent: Mutt/1.5.23.1 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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@shutemov.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)"

> >> +
> >> +     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.


-- 
 Kirill A. Shutemov