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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Thu, 14 Jan 2016 17:31:31 +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 Thu, Jan 14, 2016 at 5:25 PM, Kirill A. Shutemov
<kirill@...temov.name> wrote:
> On Thu, Jan 14, 2016 at 03:22:21PM +0100, Dmitry Vyukov wrote:
>> +The following program demonstrates kcov usage from within a test program:
>> +
>> +#include <stdio.h>
>> +#include <stddef.h>
>> +#include <stdint.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +#include <sys/ioctl.h>
>> +#include <sys/mman.h>
>> +#include <fcntl.h>
>> +
>> +#define KCOV_INIT_TRACE                      _IOR('c', 1, unsigned long)
>> +#define KCOV_ENABLE                  _IO('c', 100)
>> +#define KCOV_DISABLE                 _IO('c', 101)
>> +#define COVER_SIZE                   (64<<10)
>> +
>> +int main(int argc, char **argv)
>> +{
>> +     int fd;
>> +     uint32_t *cover, n, i;
>> +
>> +     /* A single fd descriptor allows coverage collection on a single
>> +      * thread.
>> +      */
>> +     fd = open("/sys/kernel/debug/kcov", O_RDWR);
>> +     if (fd == -1)
>> +             perror("open");
>> +     /* Setup trace mode and trace size. */
>> +     if (ioctl(fd, KCOV_INIT_TRACE, COVER_SIZE))
>> +             perror("ioctl");
>> +     /* Mmap buffer shared between kernel- and user-space. */
>> +     cover = (uint32_t*)mmap(NULL, COVER_SIZE * sizeof(uint32_t),
>> +                             PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
>> +     if ((void*)cover == MAP_FAILED)
>> +             perror("mmap");
>> +     /* Enable coverage collection on the current thread. */
>> +     if (ioctl(fd, KCOV_ENABLE, 0))
>> +             perror("ioctl");
>> +     /* Reset coverage from the tail of the ioctl() call. */
>> +     __atomic_store_n(&cover[0], 0, __ATOMIC_RELAXED);
>> +     /* That's the target syscal call. */
>> +     read(-1, NULL, 0);
>> +     /* Read number of PCs collected. */
>> +     n = __atomic_load_n(&cover[0], __ATOMIC_RELAXED);
>> +     /* PCs are shorten to uint32_t, so we need to restore the upper part. */
>> +     for (i = 0; i < n; i++)
>> +             printf("0xffffffff%0lx\n", (unsigned long)cover[i + 1]);
>> +     /* Disable coverage collection for the current thread. After this call
>> +      * coverage can be enabled for a different thread.
>> +      */
>> +     if (ioctl(fd, KCOV_DISABLE, 0))
>> +             perror("ioctl");
>> +     /* Free resources. */
>> +     if (munmap(cover, COVER_SIZE * sizeof(uint32_t)))
>> +             perror("munmap");
>> +     if (close(fd))
>> +             perror("close");
>> +     return 0;
>> +}
>
> Do we really need ioctl in this interface? Why not just plain write(2)?

Initially I had writes (similar to kmemleak interface). Quentin's
original patch used ioctl's and he said that ioctls are cleaner (no
string formatting and parsing). Note that as opposed to kmemleak, kcov
is not supposed to be used from command line. I don't care too much
either way. I guess this is matter of personal preferences to some
degree and there are precedents of both ioctls and writes...