linux-kernel - Re: [PATCH v2 1/3] kcov: support comparison operands collection

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] [day] [month] [year] [list]

Date:   Wed, 11 Oct 2017 11:56:01 +0200
From:   Alexander Potapenko <glider@...gle.com>
To:     Dmitry Vyukov <dvyukov@...gle.com>
Cc:     Mark Rutland <mark.rutland@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Alexander Popov <alex.popov@...ux.com>,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Quentin Casasnovas <quentin.casasnovas@...cle.com>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Vegard Nossum <vegard.nossum@...cle.com>,
        syzkaller <syzkaller@...glegroups.com>,
        Linux Memory Management List <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] kcov: support comparison operands collection

On Tue, Oct 10, 2017 at 5:34 PM, 'Dmitry Vyukov' via syzkaller
<syzkaller@...glegroups.com> wrote:
> On Tue, Oct 10, 2017 at 5:28 PM, 'Alexander Potapenko' via syzkaller
> <syzkaller@...glegroups.com> wrote:
>> On Mon, Oct 9, 2017 at 8:46 AM, Mark Rutland <mark.rutland@....com> wrote:
>>> Hi,
>>>
>>> I look forward to using this! :)
>>>
>>> I just have afew comments below.
>>>
>>> On Mon, Oct 09, 2017 at 05:05:19PM +0200, Alexander Potapenko wrote:
>>>> +/*
>>>> + * Defines the format for the types of collected comparisons.
>>>> + */
>>>> +enum kcov_cmp_type {
>>>> +     /*
>>>> +      * LSB shows whether one of the arguments is a compile-time constant.
>>>> +      */
>>>> +     KCOV_CMP_CONST = 1,
>>>> +     /*
>>>> +      * Second and third LSBs contain the size of arguments (1/2/4/8 bytes).
>>>> +      */
>>>> +     KCOV_CMP_SIZE1 = 0,
>>>> +     KCOV_CMP_SIZE2 = 2,
>>>> +     KCOV_CMP_SIZE4 = 4,
>>>> +     KCOV_CMP_SIZE8 = 6,
>>>> +     KCOV_CMP_SIZE_MASK = 6,
>>>> +};
>>>
>>> Given that LSB is meant to be OR-ed in, (and hence combinations of
>>> values are meaningful) I don't think it makes sense for this to be an
>>> enum. This would clearer as something like:
>>>
>>> /*
>>>  * The format for the types of collected comparisons.
>>>  *
>>>  * Bit 0 shows whether one of the arguments is a compile-time constant.
>>>  * Bits 1 & 2 contain log2 of the argument size, up to 8 bytes.
>>>  */
>>> #define KCOV_CMP_CONST          (1 << 0)
>>> #define KCOV_CMP_SIZE(n)        ((n) << 1)
>>> #define KCOV_CMP_MASK           KCOV_CMP_SIZE(3)
>> Agreed.
>>> ... I note that a few places in the kernel use a 128-bit type. Are
>>> 128-bit comparisons not instrumented?
>>>
>>> [...]
>>>
>>>> +static bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t)
>>>> +{
>>>> +     enum kcov_mode mode;
>>>> +
>>>> +     /*
>>>> +      * We are interested in code coverage as a function of a syscall inputs,
>>>> +      * so we ignore code executed in interrupts.
>>>> +      */
>>>> +     if (!t || !in_task())
>>>> +             return false;
>>>
>>> This !t check can go, as with the one in __sanitizer_cov_trace_pc, since
>>> t is always current, and therefore cannot be NULL.
>> Ok.
>>> IIRC there's a patch queued for that, which this may conflict with.
>> Sorry, I don't quite understand what exactly is conflicting here.
>
>
> This patch should be in mm tree:
> https://patchwork.kernel.org/patch/9978383/
Ok, I've rebased on top of it, see v4.
> --
> You received this message because you are subscribed to the Google Groups "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+unsubscribe@...glegroups.com.
> For more options, visit https://groups.google.com/d/optout.



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg