Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <cover.1576170740.git.andreyknvl@google.com>
In-Reply-To: <cover.1576170740.git.andreyknvl@google.com>
From:   Andrey Konovalov <andreyknvl@google.com>
Date:   Fri, 13 Dec 2019 13:11:26 +0100
Message-ID: <CAAeHK+xkvQ3Jxot15pw1_EHZgZ2i539pLSxgXujDLWO0x0DrRw@mail.gmail.com>
Subject: Re: [PATCH RFC 0/2] kcov: collect coverage from usbhid interrupts
To:     Dmitry Vyukov <dvyukov@google.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Alan Stern <stern@rowland.harvard.edu>,
        Jiri Kosina <jikos@kernel.org>,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        USB list <linux-usb@vger.kernel.org>,
        linux-input@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
        Alexander Potapenko <glider@google.com>,
        Marco Elver <elver@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Dec 12, 2019 at 6:15 PM Andrey Konovalov <andreyknvl@google.com> wrote:
>
> This patchset extends kcov to allow collecting coverage from interrupts
> and then uses the new functionality to collect coverage from usbhid code.
>
> What I'm not sure yet about this change is if we actually want to
> selectively annotate some parts of the USB stack that are executed in
> interrupt context, or maybe we can do this with some common approach.
>
> For example patch #2 in this patchset annotates all functions that are
> passed as completion callbacks to usb_fill_*() in drivers/hid/usbhid.
> Maybe instead we could redefine usb_fill_*() in a way that would handle
> all such cases without manual annotations.

Although looking at this again today, it seems much more logical to
add kcov annotations around the complete() callback in
__usb_hcd_giveback_urb(). Don't know why I didn't think of that. Will
do in the next version.


>
> Any suggestions are welcome.
>
> This has allowed to find at least one new HID bug [1], which was recently
> fixed by Alan [2].
>
> [1] https://syzkaller.appspot.com/bug?extid=09ef48aa58261464b621
> [2] https://patchwork.kernel.org/patch/11283319/
>
> This patchset has been pushed to the public Linux kernel Gerrit instance:
>
> https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/2225
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
>
> Andrey Konovalov (2):
>   kcov: collect coverage from interrupts
>   HID: usbhid: kcov: add annotations for coverage collection
>
>  Documentation/dev-tools/kcov.rst   |  16 +--
>  drivers/hid/usbhid/hid-core.c      |  25 +++-
>  drivers/hid/usbhid/usbkbd.c        |  15 ++-
>  drivers/hid/usbhid/usbmouse.c      |   7 +-
>  drivers/usb/gadget/udc/dummy_hcd.c |   1 +
>  include/linux/sched.h              |   3 +
>  kernel/kcov.c                      | 196 +++++++++++++++++++----------
>  lib/Kconfig.debug                  |   9 ++
>  8 files changed, 192 insertions(+), 80 deletions(-)
>
> --
> 2.24.1.735.g03f4e72817-goog
>