* [PATCH] KCOV: Introduced tracing unique covered PCs @ 2021-02-11 8:07 Alexander Lochmann 2021-02-12 12:54 ` Dmitry Vyukov 0 siblings, 1 reply; 13+ messages in thread From: Alexander Lochmann @ 2021-02-11 8:07 UTC (permalink / raw) Cc: Alexander Lochmann, Dmitry Vyukov, Andrey Konovalov, Jonathan Corbet, Andrew Morton, Wei Yongjun, Maciej Grochowski, kasan-dev, linux-doc, linux-kernel Introduced new tracing mode KCOV_MODE_UNIQUE. It simply stores the executed PCs. The execution order is discarded. Each bit in the shared buffer represents every fourth byte of the text segment. Since a call instruction on every supported architecture is at least four bytes, it is safe to just store every fourth byte of the text segment. In contrast to KCOV_MODE_TRACE_PC, the shared buffer cannot overflow. Thus, all executed PCs are recorded. Signed-off-by: Alexander Lochmann <info@alexander-lochmann.de> --- Documentation/dev-tools/kcov.rst | 80 ++++++++++++++++++++++++++++++++ include/linux/kcov.h | 4 +- include/uapi/linux/kcov.h | 10 ++++ kernel/kcov.c | 67 ++++++++++++++++++++------ 4 files changed, 147 insertions(+), 14 deletions(-) diff --git a/Documentation/dev-tools/kcov.rst b/Documentation/dev-tools/kcov.rst index 8548b0b04e43..4712a730a06a 100644 --- a/Documentation/dev-tools/kcov.rst +++ b/Documentation/dev-tools/kcov.rst @@ -127,6 +127,86 @@ That is, a parent process opens /sys/kernel/debug/kcov, enables trace mode, mmaps coverage buffer and then forks child processes in a loop. Child processes only need to enable coverage (disable happens automatically on thread end). +If someone is interested in a set of executed PCs, and does not care about +execution order, he or she can advise KCOV to do so: + +.. code-block:: c + + #include <stdio.h> + #include <stddef.h> + #include <stdint.h> + #include <stdlib.h> + #include <sys/types.h> + #include <sys/stat.h> + #include <sys/ioctl.h> + #include <sys/mman.h> + #include <unistd.h> + #include <fcntl.h> + + #define KCOV_INIT_TRACE _IOR('c', 1, unsigned long) + #define KCOV_INIT_UNIQUE _IOR('c', 2, unsigned long) + #define KCOV_ENABLE _IO('c', 100) + #define KCOV_DISABLE _IO('c', 101) + + #define BITS_PER_LONG 64 + #define KCOV_TRACE_PC 0 + #define KCOV_TRACE_CMP 1 + #define KCOV_UNIQUE_PC 2 + /* + * Determine start of text segment via 'nm vmlinux | grep _stext | cut -d " " -f1', + * and fill in. + */ + #define STEXT_START 0xffffffff81000000 + + + + int main(int argc, char **argv) + { + int fd; + unsigned long *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"), exit(1); + /* Setup trace mode and trace size. */ + if ((n = ioctl(fd, KCOV_INIT_UNIQUE, 0)) < 0) + perror("ioctl"), exit(1); + /* Mmap buffer shared between kernel- and user-space. */ + cover = (unsigned long*)mmap(NULL, n, + PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); + if ((void*)cover == MAP_FAILED) + perror("mmap"), exit(1); + /* Enable coverage collection on the current thread. */ + if (ioctl(fd, KCOV_ENABLE, KCOV_UNIQUE_PC)) + perror("ioctl"), exit(1); + /* That's the target syscal call. */ + read(-1, NULL, 0); + /* 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"), exit(1); + /* Convert byte size into element size */ + n /= sizeof(unsigned long); + /* Print executed PCs in sorted order */ + for (i = 0; i < n; i++) { + for (int j = 0; j < BITS_PER_LONG; j++) { + if (cover[i] & (1L << j)) { + printf("0x%jx\n", (uintmax_t)(STEXT_START + (i * BITS_PER_LONG + j) * 4)); + } + } + } + /* Free resources. */ + if (munmap(cover, n * sizeof(unsigned long))) + perror("munmap"), exit(1); + if (close(fd)) + perror("close"), exit(1); + return 0; + } + Comparison operands collection ------------------------------ diff --git a/include/linux/kcov.h b/include/linux/kcov.h index a10e84707d82..aa0c8bcf8299 100644 --- a/include/linux/kcov.h +++ b/include/linux/kcov.h @@ -19,7 +19,9 @@ enum kcov_mode { */ KCOV_MODE_TRACE_PC = 2, /* Collecting comparison operands mode. */ - KCOV_MODE_TRACE_CMP = 3, + KCOV_MODE_TRACE_CMP = 4, + /* Collecting unique covered PCs. Execution order is not saved. */ + KCOV_MODE_UNIQUE_PC = 8, }; #define KCOV_IN_CTXSW (1 << 30) diff --git a/include/uapi/linux/kcov.h b/include/uapi/linux/kcov.h index 1d0350e44ae3..5b99b6d1a1ac 100644 --- a/include/uapi/linux/kcov.h +++ b/include/uapi/linux/kcov.h @@ -19,6 +19,7 @@ struct kcov_remote_arg { #define KCOV_REMOTE_MAX_HANDLES 0x100 #define KCOV_INIT_TRACE _IOR('c', 1, unsigned long) +#define KCOV_INIT_UNIQUE _IOR('c', 2, unsigned long) #define KCOV_ENABLE _IO('c', 100) #define KCOV_DISABLE _IO('c', 101) #define KCOV_REMOTE_ENABLE _IOW('c', 102, struct kcov_remote_arg) @@ -35,6 +36,15 @@ enum { KCOV_TRACE_PC = 0, /* Collecting comparison operands mode. */ KCOV_TRACE_CMP = 1, + /* + * Unique coverage collection mode. + * Unique covered PCs are collected in a per-task buffer. + * De-duplicates the collected PCs. Execution order is *not* saved. + * Each bit in the buffer represents every fourth byte of the text segment. + * Since a call instruction is at least four bytes on every supported + * architecture, storing just every fourth byte is sufficient. + */ + KCOV_UNIQUE_PC = 2, }; /* diff --git a/kernel/kcov.c b/kernel/kcov.c index 6b8368be89c8..8f00ba6e672a 100644 --- a/kernel/kcov.c +++ b/kernel/kcov.c @@ -24,6 +24,7 @@ #include <linux/refcount.h> #include <linux/log2.h> #include <asm/setup.h> +#include <asm/sections.h> #define kcov_debug(fmt, ...) pr_debug("%s: " fmt, __func__, ##__VA_ARGS__) @@ -171,7 +172,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_stru * kcov_start(). */ barrier(); - return mode == needed_mode; + return (mode & needed_mode) && !(mode & KCOV_IN_CTXSW); } static notrace unsigned long canonicalize_ip(unsigned long ip) @@ -191,18 +192,26 @@ void notrace __sanitizer_cov_trace_pc(void) struct task_struct *t; unsigned long *area; unsigned long ip = canonicalize_ip(_RET_IP_); - unsigned long pos; + unsigned long pos, idx; t = current; - if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t)) + if (!check_kcov_mode(KCOV_MODE_TRACE_PC | KCOV_MODE_UNIQUE_PC, t)) return; area = t->kcov_area; - /* The first 64-bit word is the number of subsequent PCs. */ - pos = READ_ONCE(area[0]) + 1; - if (likely(pos < t->kcov_size)) { - area[pos] = ip; - WRITE_ONCE(area[0], pos); + if (likely(t->kcov_mode == KCOV_MODE_TRACE_PC)) { + /* The first 64-bit word is the number of subsequent PCs. */ + pos = READ_ONCE(area[0]) + 1; + if (likely(pos < t->kcov_size)) { + area[pos] = ip; + WRITE_ONCE(area[0], pos); + } + } else { + idx = (ip - canonicalize_ip((unsigned long)&_stext)) / 4; + pos = idx % BITS_PER_LONG; + idx /= BITS_PER_LONG; + if (likely(idx < t->kcov_size)) + WRITE_ONCE(area[idx], READ_ONCE(area[idx]) | 1L << pos); } } EXPORT_SYMBOL(__sanitizer_cov_trace_pc); @@ -474,6 +483,7 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma) goto exit; } if (!kcov->area) { + kcov_debug("mmap(): Allocating 0x%lx bytes\n", size); kcov->area = area; vma->vm_flags |= VM_DONTEXPAND; spin_unlock_irqrestore(&kcov->lock, flags); @@ -515,6 +525,8 @@ static int kcov_get_mode(unsigned long arg) { if (arg == KCOV_TRACE_PC) return KCOV_MODE_TRACE_PC; + else if (arg == KCOV_UNIQUE_PC) + return KCOV_MODE_UNIQUE_PC; else if (arg == KCOV_TRACE_CMP) #ifdef CONFIG_KCOV_ENABLE_COMPARISONS return KCOV_MODE_TRACE_CMP; @@ -562,12 +574,13 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, { struct task_struct *t; unsigned long size, unused; - int mode, i; + int mode, i, text_size, ret = 0; struct kcov_remote_arg *remote_arg; struct kcov_remote *remote; unsigned long flags; switch (cmd) { + case KCOV_INIT_UNIQUE: case KCOV_INIT_TRACE: /* * Enable kcov in trace mode and setup buffer size. @@ -581,11 +594,39 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, * that must not overflow. */ size = arg; - if (size < 2 || size > INT_MAX / sizeof(unsigned long)) - return -EINVAL; - kcov->size = size; + if (cmd == KCOV_INIT_UNIQUE) { + if (size != 0) + return -EINVAL; + text_size = (canonicalize_ip((unsigned long)&_etext) - canonicalize_ip((unsigned long)&_stext)); + /** + * A call instr is at least four bytes on every supported architecture. + * Hence, just every fourth instruction can potentially be a call. + */ + text_size /= 4; + /* + * Round up size of text segment to multiple of BITS_PER_LONG. + * Otherwise, we cannot track + * the last (text_size % BITS_PER_LONG) addresses. + */ + text_size = roundup(text_size, BITS_PER_LONG); + /* Get the amount of bytes needed */ + text_size = text_size / 8; + /* mmap() requires size to be a multiple of PAGE_SIZE */ + text_size = roundup(text_size, PAGE_SIZE); + /* Get the cover size (= amount of longs stored) */ + ret = text_size; + kcov->size = text_size / sizeof(unsigned long); + kcov_debug("text size = 0x%lx, roundup = 0x%x, kcov->size = 0x%x\n", + ((unsigned long)&_etext) - ((unsigned long)&_stext), + text_size, + kcov->size); + } else { + if (size < 2 || size > INT_MAX / sizeof(unsigned long)) + return -EINVAL; + kcov->size = size; + } kcov->mode = KCOV_MODE_INIT; - return 0; + return ret; case KCOV_ENABLE: /* * Enable coverage for the current task. -- 2.30.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] KCOV: Introduced tracing unique covered PCs 2021-02-11 8:07 [PATCH] KCOV: Introduced tracing unique covered PCs Alexander Lochmann @ 2021-02-12 12:54 ` Dmitry Vyukov 2021-03-14 21:29 ` Alexander Lochmann 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2021-02-12 12:54 UTC (permalink / raw) To: info Cc: Andrey Konovalov, Jonathan Corbet, Andrew Morton, Wei Yongjun, Maciej Grochowski, kasan-dev, open list:DOCUMENTATION, LKML, syzkaller On Thu, Feb 11, 2021 at 9:07 AM Alexander Lochmann <info@alexander-lochmann.de> wrote: > > Introduced new tracing mode KCOV_MODE_UNIQUE. > It simply stores the executed PCs. > The execution order is discarded. > Each bit in the shared buffer represents every fourth > byte of the text segment. > Since a call instruction on every supported > architecture is at least four bytes, it is safe > to just store every fourth byte of the text segment. > In contrast to KCOV_MODE_TRACE_PC, the shared buffer > cannot overflow. Thus, all executed PCs are recorded. > > Signed-off-by: Alexander Lochmann <info@alexander-lochmann.de> > --- > Documentation/dev-tools/kcov.rst | 80 ++++++++++++++++++++++++++++++++ > include/linux/kcov.h | 4 +- > include/uapi/linux/kcov.h | 10 ++++ > kernel/kcov.c | 67 ++++++++++++++++++++------ > 4 files changed, 147 insertions(+), 14 deletions(-) > > diff --git a/Documentation/dev-tools/kcov.rst b/Documentation/dev-tools/kcov.rst > index 8548b0b04e43..4712a730a06a 100644 > --- a/Documentation/dev-tools/kcov.rst > +++ b/Documentation/dev-tools/kcov.rst > @@ -127,6 +127,86 @@ That is, a parent process opens /sys/kernel/debug/kcov, enables trace mode, > mmaps coverage buffer and then forks child processes in a loop. Child processes > only need to enable coverage (disable happens automatically on thread end). > > +If someone is interested in a set of executed PCs, and does not care about > +execution order, he or she can advise KCOV to do so: > + > +.. code-block:: c > + > + #include <stdio.h> > + #include <stddef.h> > + #include <stdint.h> > + #include <stdlib.h> > + #include <sys/types.h> > + #include <sys/stat.h> > + #include <sys/ioctl.h> > + #include <sys/mman.h> > + #include <unistd.h> > + #include <fcntl.h> > + > + #define KCOV_INIT_TRACE _IOR('c', 1, unsigned long) > + #define KCOV_INIT_UNIQUE _IOR('c', 2, unsigned long) > + #define KCOV_ENABLE _IO('c', 100) > + #define KCOV_DISABLE _IO('c', 101) > + > + #define BITS_PER_LONG 64 > + #define KCOV_TRACE_PC 0 > + #define KCOV_TRACE_CMP 1 > + #define KCOV_UNIQUE_PC 2 > + /* > + * Determine start of text segment via 'nm vmlinux | grep _stext | cut -d " " -f1', > + * and fill in. > + */ > + #define STEXT_START 0xffffffff81000000 > + > + > + > + int main(int argc, char **argv) > + { > + int fd; > + unsigned long *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"), exit(1); > + /* Setup trace mode and trace size. */ > + if ((n = ioctl(fd, KCOV_INIT_UNIQUE, 0)) < 0) > + perror("ioctl"), exit(1); > + /* Mmap buffer shared between kernel- and user-space. */ > + cover = (unsigned long*)mmap(NULL, n, > + PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); > + if ((void*)cover == MAP_FAILED) > + perror("mmap"), exit(1); > + /* Enable coverage collection on the current thread. */ > + if (ioctl(fd, KCOV_ENABLE, KCOV_UNIQUE_PC)) > + perror("ioctl"), exit(1); > + /* That's the target syscal call. */ > + read(-1, NULL, 0); > + /* 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"), exit(1); > + /* Convert byte size into element size */ > + n /= sizeof(unsigned long); > + /* Print executed PCs in sorted order */ > + for (i = 0; i < n; i++) { > + for (int j = 0; j < BITS_PER_LONG; j++) { > + if (cover[i] & (1L << j)) { > + printf("0x%jx\n", (uintmax_t)(STEXT_START + (i * BITS_PER_LONG + j) * 4)); > + } > + } > + } > + /* Free resources. */ > + if (munmap(cover, n * sizeof(unsigned long))) > + perror("munmap"), exit(1); > + if (close(fd)) > + perror("close"), exit(1); > + return 0; > + } > + > Comparison operands collection > ------------------------------ > > diff --git a/include/linux/kcov.h b/include/linux/kcov.h > index a10e84707d82..aa0c8bcf8299 100644 > --- a/include/linux/kcov.h > +++ b/include/linux/kcov.h > @@ -19,7 +19,9 @@ enum kcov_mode { > */ > KCOV_MODE_TRACE_PC = 2, > /* Collecting comparison operands mode. */ > - KCOV_MODE_TRACE_CMP = 3, > + KCOV_MODE_TRACE_CMP = 4, > + /* Collecting unique covered PCs. Execution order is not saved. */ > + KCOV_MODE_UNIQUE_PC = 8, > }; > > #define KCOV_IN_CTXSW (1 << 30) > diff --git a/include/uapi/linux/kcov.h b/include/uapi/linux/kcov.h > index 1d0350e44ae3..5b99b6d1a1ac 100644 > --- a/include/uapi/linux/kcov.h > +++ b/include/uapi/linux/kcov.h > @@ -19,6 +19,7 @@ struct kcov_remote_arg { > #define KCOV_REMOTE_MAX_HANDLES 0x100 > > #define KCOV_INIT_TRACE _IOR('c', 1, unsigned long) > +#define KCOV_INIT_UNIQUE _IOR('c', 2, unsigned long) > #define KCOV_ENABLE _IO('c', 100) > #define KCOV_DISABLE _IO('c', 101) > #define KCOV_REMOTE_ENABLE _IOW('c', 102, struct kcov_remote_arg) > @@ -35,6 +36,15 @@ enum { > KCOV_TRACE_PC = 0, > /* Collecting comparison operands mode. */ > KCOV_TRACE_CMP = 1, > + /* > + * Unique coverage collection mode. > + * Unique covered PCs are collected in a per-task buffer. > + * De-duplicates the collected PCs. Execution order is *not* saved. > + * Each bit in the buffer represents every fourth byte of the text segment. > + * Since a call instruction is at least four bytes on every supported > + * architecture, storing just every fourth byte is sufficient. > + */ > + KCOV_UNIQUE_PC = 2, > }; > > /* > diff --git a/kernel/kcov.c b/kernel/kcov.c > index 6b8368be89c8..8f00ba6e672a 100644 > --- a/kernel/kcov.c > +++ b/kernel/kcov.c > @@ -24,6 +24,7 @@ > #include <linux/refcount.h> > #include <linux/log2.h> > #include <asm/setup.h> > +#include <asm/sections.h> > > #define kcov_debug(fmt, ...) pr_debug("%s: " fmt, __func__, ##__VA_ARGS__) > > @@ -171,7 +172,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_stru > * kcov_start(). > */ > barrier(); > - return mode == needed_mode; > + return (mode & needed_mode) && !(mode & KCOV_IN_CTXSW); I see this produces an additional check and branch: void foo1(unsigned mode) { if ((mode & 10) && !(mode & (1<<30))) foo(); } 0: 40 f6 c7 0a test $0xa,%dil 4: 74 0f je 15 <foo1+0x15> 6: 81 e7 00 00 00 40 and $0x40000000,%edi c: 75 07 jne 15 <foo1+0x15> e: 31 c0 xor %eax,%eax 10: e9 00 00 00 00 jmpq 15 <foo1+0x15> I think we could make KCOV_IN_CTXSW sign bit and then express the check as: void foo2(unsigned mode) { if (((int)(mode & 0x8000000a)) > 0) foo(); } 0000000000000020 <foo2>: 20: 81 e7 0a 00 00 80 and $0x8000000a,%edi 26: 7f 08 jg 30 <foo2+0x10> 28: c3 retq > } > > static notrace unsigned long canonicalize_ip(unsigned long ip) > @@ -191,18 +192,26 @@ void notrace __sanitizer_cov_trace_pc(void) > struct task_struct *t; > unsigned long *area; > unsigned long ip = canonicalize_ip(_RET_IP_); > - unsigned long pos; > + unsigned long pos, idx; > > t = current; > - if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t)) > + if (!check_kcov_mode(KCOV_MODE_TRACE_PC | KCOV_MODE_UNIQUE_PC, t)) > return; > > area = t->kcov_area; > - /* The first 64-bit word is the number of subsequent PCs. */ > - pos = READ_ONCE(area[0]) + 1; > - if (likely(pos < t->kcov_size)) { > - area[pos] = ip; > - WRITE_ONCE(area[0], pos); > + if (likely(t->kcov_mode == KCOV_MODE_TRACE_PC)) { Does this introduce an additional real of t->kcov_mode? If yes, please reuse the value read in check_kcov_mode. > + /* The first 64-bit word is the number of subsequent PCs. */ > + pos = READ_ONCE(area[0]) + 1; > + if (likely(pos < t->kcov_size)) { > + area[pos] = ip; > + WRITE_ONCE(area[0], pos); > + } > + } else { > + idx = (ip - canonicalize_ip((unsigned long)&_stext)) / 4; > + pos = idx % BITS_PER_LONG; > + idx /= BITS_PER_LONG; > + if (likely(idx < t->kcov_size)) > + WRITE_ONCE(area[idx], READ_ONCE(area[idx]) | 1L << pos); > } > } > EXPORT_SYMBOL(__sanitizer_cov_trace_pc); > @@ -474,6 +483,7 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma) > goto exit; > } > if (!kcov->area) { > + kcov_debug("mmap(): Allocating 0x%lx bytes\n", size); > kcov->area = area; > vma->vm_flags |= VM_DONTEXPAND; > spin_unlock_irqrestore(&kcov->lock, flags); > @@ -515,6 +525,8 @@ static int kcov_get_mode(unsigned long arg) > { > if (arg == KCOV_TRACE_PC) > return KCOV_MODE_TRACE_PC; > + else if (arg == KCOV_UNIQUE_PC) > + return KCOV_MODE_UNIQUE_PC; As far as I understand, users can first do KCOV_INIT_UNIQUE and then enable KCOV_TRACE_PC, or vice versa. It looks somewhat strange. Is it intentional? It's not possible to specify buffer size for KCOV_INIT_UNIQUE, so most likely the buffer will be either too large or too small for a trace. > else if (arg == KCOV_TRACE_CMP) > #ifdef CONFIG_KCOV_ENABLE_COMPARISONS > return KCOV_MODE_TRACE_CMP; > @@ -562,12 +574,13 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, > { > struct task_struct *t; > unsigned long size, unused; > - int mode, i; > + int mode, i, text_size, ret = 0; > struct kcov_remote_arg *remote_arg; > struct kcov_remote *remote; > unsigned long flags; > > switch (cmd) { > + case KCOV_INIT_UNIQUE: I think nowadays you need some annotation like fallthrough here. > case KCOV_INIT_TRACE: > /* > * Enable kcov in trace mode and setup buffer size. > @@ -581,11 +594,39 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, > * that must not overflow. > */ > size = arg; > - if (size < 2 || size > INT_MAX / sizeof(unsigned long)) > - return -EINVAL; > - kcov->size = size; > + if (cmd == KCOV_INIT_UNIQUE) { > + if (size != 0) > + return -EINVAL; > + text_size = (canonicalize_ip((unsigned long)&_etext) - canonicalize_ip((unsigned long)&_stext)); > + /** > + * A call instr is at least four bytes on every supported architecture. > + * Hence, just every fourth instruction can potentially be a call. > + */ > + text_size /= 4; Strictly saying, we need to round up text_size to 4 before dividing by 4. Otherwise we potentially don't cover up to the last 3 bytes. > + /* > + * Round up size of text segment to multiple of BITS_PER_LONG. > + * Otherwise, we cannot track > + * the last (text_size % BITS_PER_LONG) addresses. > + */ > + text_size = roundup(text_size, BITS_PER_LONG); > + /* Get the amount of bytes needed */ > + text_size = text_size / 8; > + /* mmap() requires size to be a multiple of PAGE_SIZE */ > + text_size = roundup(text_size, PAGE_SIZE); > + /* Get the cover size (= amount of longs stored) */ s/longs/bytes/ > + ret = text_size; > + kcov->size = text_size / sizeof(unsigned long); > + kcov_debug("text size = 0x%lx, roundup = 0x%x, kcov->size = 0x%x\n", > + ((unsigned long)&_etext) - ((unsigned long)&_stext), > + text_size, > + kcov->size); > + } else { > + if (size < 2 || size > INT_MAX / sizeof(unsigned long)) > + return -EINVAL; > + kcov->size = size; > + } > kcov->mode = KCOV_MODE_INIT; > - return 0; > + return ret; > case KCOV_ENABLE: > /* > * Enable coverage for the current task. > -- > 2.30.0 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] KCOV: Introduced tracing unique covered PCs 2021-02-12 12:54 ` Dmitry Vyukov @ 2021-03-14 21:29 ` Alexander Lochmann 2021-03-15 8:02 ` Dmitry Vyukov 0 siblings, 1 reply; 13+ messages in thread From: Alexander Lochmann @ 2021-03-14 21:29 UTC (permalink / raw) To: Dmitry Vyukov Cc: Andrey Konovalov, Jonathan Corbet, Andrew Morton, Wei Yongjun, Maciej Grochowski, kasan-dev, open list:DOCUMENTATION, LKML, syzkaller On 12.02.21 13:54, Dmitry Vyukov wrote: > > I think we could make KCOV_IN_CTXSW sign bit and then express the check as: > > void foo2(unsigned mode) { > if (((int)(mode & 0x8000000a)) > 0) > foo(); > } > > 0000000000000020 <foo2>: > 20: 81 e7 0a 00 00 80 and $0x8000000a,%edi > 26: 7f 08 jg 30 <foo2+0x10> > 28: c3 retq > So ((int)(mode & (KCOV_IN_CTXSW | needed_mode))) > 0? > > > >> } >> >> static notrace unsigned long canonicalize_ip(unsigned long ip) >> @@ -191,18 +192,26 @@ void notrace __sanitizer_cov_trace_pc(void) >> struct task_struct *t; >> unsigned long *area; >> unsigned long ip = canonicalize_ip(_RET_IP_); >> - unsigned long pos; >> + unsigned long pos, idx; >> >> t = current; >> - if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t)) >> + if (!check_kcov_mode(KCOV_MODE_TRACE_PC | KCOV_MODE_UNIQUE_PC, t)) >> return; >> >> area = t->kcov_area; >> - /* The first 64-bit word is the number of subsequent PCs. */ >> - pos = READ_ONCE(area[0]) + 1; >> - if (likely(pos < t->kcov_size)) { >> - area[pos] = ip; >> - WRITE_ONCE(area[0], pos); >> + if (likely(t->kcov_mode == KCOV_MODE_TRACE_PC)) { > > Does this introduce an additional real of t->kcov_mode? > If yes, please reuse the value read in check_kcov_mode. Okay. How do I get that value from check_kcov_mode() to the caller? Shall I add an additional parameter to check_kcov_mode()? > > >> + /* The first 64-bit word is the number of subsequent PCs. */ >> + pos = READ_ONCE(area[0]) + 1; >> + if (likely(pos < t->kcov_size)) { >> + area[pos] = ip; >> + WRITE_ONCE(area[0], pos); >> + } >> + } else { >> + idx = (ip - canonicalize_ip((unsigned long)&_stext)) / 4; >> + pos = idx % BITS_PER_LONG; >> + idx /= BITS_PER_LONG; >> + if (likely(idx < t->kcov_size)) >> + WRITE_ONCE(area[idx], READ_ONCE(area[idx]) | 1L << pos); >> } >> } >> EXPORT_SYMBOL(__sanitizer_cov_trace_pc); >> @@ -474,6 +483,7 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma) >> goto exit; >> } >> if (!kcov->area) { >> + kcov_debug("mmap(): Allocating 0x%lx bytes\n", size); >> kcov->area = area; >> vma->vm_flags |= VM_DONTEXPAND; >> spin_unlock_irqrestore(&kcov->lock, flags); >> @@ -515,6 +525,8 @@ static int kcov_get_mode(unsigned long arg) >> { >> if (arg == KCOV_TRACE_PC) >> return KCOV_MODE_TRACE_PC; >> + else if (arg == KCOV_UNIQUE_PC) >> + return KCOV_MODE_UNIQUE_PC; > > As far as I understand, users can first do KCOV_INIT_UNIQUE and then > enable KCOV_TRACE_PC, or vice versa. > It looks somewhat strange. Is it intentional? I'll fix that. It's not possible to > specify buffer size for KCOV_INIT_UNIQUE, so most likely the buffer > will be either too large or too small for a trace. No, the buffer will be calculated by KCOV_INIT_UNIQUE based on the size of the text segment. - Alex -- Alexander Lochmann PGP key: 0xBC3EF6FD Heiliger Weg 72 phone: +49.231.28053964 D-44141 Dortmund mobile: +49.151.15738323 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] KCOV: Introduced tracing unique covered PCs 2021-03-14 21:29 ` Alexander Lochmann @ 2021-03-15 8:02 ` Dmitry Vyukov 2021-03-15 21:43 ` Alexander Lochmann 2021-03-17 21:10 ` Alexander Lochmann 0 siblings, 2 replies; 13+ messages in thread From: Dmitry Vyukov @ 2021-03-15 8:02 UTC (permalink / raw) To: Alexander Lochmann Cc: Andrey Konovalov, Jonathan Corbet, Andrew Morton, Wei Yongjun, Maciej Grochowski, kasan-dev, open list:DOCUMENTATION, LKML, syzkaller On Sun, Mar 14, 2021 at 10:29 PM Alexander Lochmann <info@alexander-lochmann.de> wrote: > On 12.02.21 13:54, Dmitry Vyukov wrote: > > > > I think we could make KCOV_IN_CTXSW sign bit and then express the check as: > > > > void foo2(unsigned mode) { > > if (((int)(mode & 0x8000000a)) > 0) > > foo(); > > } > > > > 0000000000000020 <foo2>: > > 20: 81 e7 0a 00 00 80 and $0x8000000a,%edi > > 26: 7f 08 jg 30 <foo2+0x10> > > 28: c3 retq > > > So ((int)(mode & (KCOV_IN_CTXSW | needed_mode))) > 0? Frankly I lost all context now. If it results in optimal code, then, yes. > >> } > >> > >> static notrace unsigned long canonicalize_ip(unsigned long ip) > >> @@ -191,18 +192,26 @@ void notrace __sanitizer_cov_trace_pc(void) > >> struct task_struct *t; > >> unsigned long *area; > >> unsigned long ip = canonicalize_ip(_RET_IP_); > >> - unsigned long pos; > >> + unsigned long pos, idx; > >> > >> t = current; > >> - if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t)) > >> + if (!check_kcov_mode(KCOV_MODE_TRACE_PC | KCOV_MODE_UNIQUE_PC, t)) > >> return; > >> > >> area = t->kcov_area; > >> - /* The first 64-bit word is the number of subsequent PCs. */ > >> - pos = READ_ONCE(area[0]) + 1; > >> - if (likely(pos < t->kcov_size)) { > >> - area[pos] = ip; > >> - WRITE_ONCE(area[0], pos); > >> + if (likely(t->kcov_mode == KCOV_MODE_TRACE_PC)) { > > > > Does this introduce an additional real of t->kcov_mode? > > If yes, please reuse the value read in check_kcov_mode. > Okay. How do I get that value from check_kcov_mode() to the caller? > Shall I add an additional parameter to check_kcov_mode()? Yes, I would try to add an additional pointer parameter for mode. I think after inlining the compiler should be able to regestrize it. > >> + /* The first 64-bit word is the number of subsequent PCs. */ > >> + pos = READ_ONCE(area[0]) + 1; > >> + if (likely(pos < t->kcov_size)) { > >> + area[pos] = ip; > >> + WRITE_ONCE(area[0], pos); > >> + } > >> + } else { > >> + idx = (ip - canonicalize_ip((unsigned long)&_stext)) / 4; > >> + pos = idx % BITS_PER_LONG; > >> + idx /= BITS_PER_LONG; > >> + if (likely(idx < t->kcov_size)) > >> + WRITE_ONCE(area[idx], READ_ONCE(area[idx]) | 1L << pos); > >> } > >> } > >> EXPORT_SYMBOL(__sanitizer_cov_trace_pc); > >> @@ -474,6 +483,7 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma) > >> goto exit; > >> } > >> if (!kcov->area) { > >> + kcov_debug("mmap(): Allocating 0x%lx bytes\n", size); > >> kcov->area = area; > >> vma->vm_flags |= VM_DONTEXPAND; > >> spin_unlock_irqrestore(&kcov->lock, flags); > >> @@ -515,6 +525,8 @@ static int kcov_get_mode(unsigned long arg) > >> { > >> if (arg == KCOV_TRACE_PC) > >> return KCOV_MODE_TRACE_PC; > >> + else if (arg == KCOV_UNIQUE_PC) > >> + return KCOV_MODE_UNIQUE_PC; > > > > As far as I understand, users can first do KCOV_INIT_UNIQUE and then > > enable KCOV_TRACE_PC, or vice versa. > > It looks somewhat strange. Is it intentional? > I'll fix that. > It's not possible to > > specify buffer size for KCOV_INIT_UNIQUE, so most likely the buffer > > will be either too large or too small for a trace. > No, the buffer will be calculated by KCOV_INIT_UNIQUE based on the size > of the text segment. Yes, which will be either too large or too small for KCOV_TRACE_PC enabled later. > - Alex > > -- > Alexander Lochmann PGP key: 0xBC3EF6FD > Heiliger Weg 72 phone: +49.231.28053964 > D-44141 Dortmund mobile: +49.151.15738323 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] KCOV: Introduced tracing unique covered PCs 2021-03-15 8:02 ` Dmitry Vyukov @ 2021-03-15 21:43 ` Alexander Lochmann 2021-03-16 6:36 ` Dmitry Vyukov 2021-03-17 21:10 ` Alexander Lochmann 1 sibling, 1 reply; 13+ messages in thread From: Alexander Lochmann @ 2021-03-15 21:43 UTC (permalink / raw) To: Dmitry Vyukov Cc: Andrey Konovalov, Jonathan Corbet, Andrew Morton, Wei Yongjun, Maciej Grochowski, kasan-dev, open list:DOCUMENTATION, LKML, syzkaller On 15.03.21 09:02, Dmitry Vyukov wrote: >>>> static notrace unsigned long canonicalize_ip(unsigned long ip) >>>> @@ -191,18 +192,26 @@ void notrace __sanitizer_cov_trace_pc(void) >>>> struct task_struct *t; >>>> unsigned long *area; >>>> unsigned long ip = canonicalize_ip(_RET_IP_); >>>> - unsigned long pos; >>>> + unsigned long pos, idx; >>>> >>>> t = current; >>>> - if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t)) >>>> + if (!check_kcov_mode(KCOV_MODE_TRACE_PC | KCOV_MODE_UNIQUE_PC, t)) >>>> return; >>>> >>>> area = t->kcov_area; >>>> - /* The first 64-bit word is the number of subsequent PCs. */ >>>> - pos = READ_ONCE(area[0]) + 1; >>>> - if (likely(pos < t->kcov_size)) { >>>> - area[pos] = ip; >>>> - WRITE_ONCE(area[0], pos); >>>> + if (likely(t->kcov_mode == KCOV_MODE_TRACE_PC)) { >>> >>> Does this introduce an additional real of t->kcov_mode? >>> If yes, please reuse the value read in check_kcov_mode. >> Okay. How do I get that value from check_kcov_mode() to the caller? >> Shall I add an additional parameter to check_kcov_mode()? > > Yes, I would try to add an additional pointer parameter for mode. I > think after inlining the compiler should be able to regestrize it. > Should kcov->mode be written directly to that ptr? Otherwise, it must be written to the already present variable mode, and than copied to the ptr (if not NULL). - Alex -- Alexander Lochmann PGP key: 0xBC3EF6FD Heiliger Weg 72 phone: +49.231.28053964 D-44141 Dortmund mobile: +49.151.15738323 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] KCOV: Introduced tracing unique covered PCs 2021-03-15 21:43 ` Alexander Lochmann @ 2021-03-16 6:36 ` Dmitry Vyukov 0 siblings, 0 replies; 13+ messages in thread From: Dmitry Vyukov @ 2021-03-16 6:36 UTC (permalink / raw) To: Alexander Lochmann Cc: Andrey Konovalov, Jonathan Corbet, Andrew Morton, Wei Yongjun, Maciej Grochowski, kasan-dev, open list:DOCUMENTATION, LKML, syzkaller On Mon, Mar 15, 2021 at 10:43 PM Alexander Lochmann <info@alexander-lochmann.de> wrote: > On 15.03.21 09:02, Dmitry Vyukov wrote: > >>>> static notrace unsigned long canonicalize_ip(unsigned long ip) > >>>> @@ -191,18 +192,26 @@ void notrace __sanitizer_cov_trace_pc(void) > >>>> struct task_struct *t; > >>>> unsigned long *area; > >>>> unsigned long ip = canonicalize_ip(_RET_IP_); > >>>> - unsigned long pos; > >>>> + unsigned long pos, idx; > >>>> > >>>> t = current; > >>>> - if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t)) > >>>> + if (!check_kcov_mode(KCOV_MODE_TRACE_PC | KCOV_MODE_UNIQUE_PC, t)) > >>>> return; > >>>> > >>>> area = t->kcov_area; > >>>> - /* The first 64-bit word is the number of subsequent PCs. */ > >>>> - pos = READ_ONCE(area[0]) + 1; > >>>> - if (likely(pos < t->kcov_size)) { > >>>> - area[pos] = ip; > >>>> - WRITE_ONCE(area[0], pos); > >>>> + if (likely(t->kcov_mode == KCOV_MODE_TRACE_PC)) { > >>> > >>> Does this introduce an additional real of t->kcov_mode? > >>> If yes, please reuse the value read in check_kcov_mode. > >> Okay. How do I get that value from check_kcov_mode() to the caller? > >> Shall I add an additional parameter to check_kcov_mode()? > > > > Yes, I would try to add an additional pointer parameter for mode. I > > think after inlining the compiler should be able to regestrize it. > > > Should kcov->mode be written directly to that ptr? > Otherwise, it must be written to the already present variable mode, and > than copied to the ptr (if not NULL). I would expect that after inlining it won't make difference in generated code. Is so, both options are fine. Whatever leads to a cleaner code. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] KCOV: Introduced tracing unique covered PCs 2021-03-15 8:02 ` Dmitry Vyukov 2021-03-15 21:43 ` Alexander Lochmann @ 2021-03-17 21:10 ` Alexander Lochmann 2021-03-18 7:17 ` Dmitry Vyukov 1 sibling, 1 reply; 13+ messages in thread From: Alexander Lochmann @ 2021-03-17 21:10 UTC (permalink / raw) To: Dmitry Vyukov Cc: Andrey Konovalov, Jonathan Corbet, Andrew Morton, Wei Yongjun, Maciej Grochowski, kasan-dev, open list:DOCUMENTATION, LKML, syzkaller On 15.03.21 09:02, Dmitry Vyukov wrote: >>> Does this introduce an additional real of t->kcov_mode? >>> If yes, please reuse the value read in check_kcov_mode. >> Okay. How do I get that value from check_kcov_mode() to the caller? >> Shall I add an additional parameter to check_kcov_mode()? > > Yes, I would try to add an additional pointer parameter for mode. I > think after inlining the compiler should be able to regestrize it. First, I'll go for the extra argument. However, the compiler doesn't seem to inline check_kcov_mode(). Can I enforce inlining? I'm using GCC 9.3 on Debian Testing. - Alex -- Alexander Lochmann PGP key: 0xBC3EF6FD Heiliger Weg 72 phone: +49.231.28053964 D-44141 Dortmund mobile: +49.151.15738323 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] KCOV: Introduced tracing unique covered PCs 2021-03-17 21:10 ` Alexander Lochmann @ 2021-03-18 7:17 ` Dmitry Vyukov 2021-03-21 18:43 ` [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE Alexander Lochmann 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2021-03-18 7:17 UTC (permalink / raw) To: Alexander Lochmann Cc: Andrey Konovalov, Jonathan Corbet, Andrew Morton, Wei Yongjun, Maciej Grochowski, kasan-dev, open list:DOCUMENTATION, LKML, syzkaller On Wed, Mar 17, 2021 at 10:10 PM Alexander Lochmann <info@alexander-lochmann.de> wrote: > On 15.03.21 09:02, Dmitry Vyukov wrote: > >>> Does this introduce an additional real of t->kcov_mode? > >>> If yes, please reuse the value read in check_kcov_mode. > >> Okay. How do I get that value from check_kcov_mode() to the caller? > >> Shall I add an additional parameter to check_kcov_mode()? > > > > Yes, I would try to add an additional pointer parameter for mode. I > > think after inlining the compiler should be able to regestrize it. > First, I'll go for the extra argument. However, the compiler doesn't > seem to inline check_kcov_mode(). Can I enforce inlining? > I'm using GCC 9.3 on Debian Testing. That's very strange and wrong. Maybe you use something like CONFIG_CC_OPTIMIZE_FOR_SIZE=y? With gcc-10 I am getting: ffffffff81529ba0 <__sanitizer_cov_trace_pc>: ffffffff81529ba0: 65 8b 05 59 53 af 7e mov %gs:0x7eaf5359(%rip),%eax # 1ef00 <__preempt_count> ffffffff81529ba7: 89 c1 mov %eax,%ecx ffffffff81529ba9: 48 8b 34 24 mov (%rsp),%rsi ffffffff81529bad: 81 e1 00 01 00 00 and $0x100,%ecx ffffffff81529bb3: 65 48 8b 14 25 40 ef mov %gs:0x1ef40,%rdx ffffffff81529bba: 01 00 ffffffff81529bbc: a9 00 01 ff 00 test $0xff0100,%eax ffffffff81529bc1: 74 0e je ffffffff81529bd1 <__sanitizer_cov_trace_pc+0x31> ffffffff81529bc3: 85 c9 test %ecx,%ecx ffffffff81529bc5: 74 35 je ffffffff81529bfc <__sanitizer_cov_trace_pc+0x5c> ffffffff81529bc7: 8b 82 d4 14 00 00 mov 0x14d4(%rdx),%eax ffffffff81529bcd: 85 c0 test %eax,%eax ffffffff81529bcf: 74 2b je ffffffff81529bfc <__sanitizer_cov_trace_pc+0x5c> ffffffff81529bd1: 8b 82 b0 14 00 00 mov 0x14b0(%rdx),%eax ffffffff81529bd7: 83 f8 02 cmp $0x2,%eax ffffffff81529bda: 75 20 jne ffffffff81529bfc <__sanitizer_cov_trace_pc+0x5c> ffffffff81529bdc: 48 8b 8a b8 14 00 00 mov 0x14b8(%rdx),%rcx ffffffff81529be3: 8b 92 b4 14 00 00 mov 0x14b4(%rdx),%edx ffffffff81529be9: 48 8b 01 mov (%rcx),%rax ffffffff81529bec: 48 83 c0 01 add $0x1,%rax ffffffff81529bf0: 48 39 c2 cmp %rax,%rdx ffffffff81529bf3: 76 07 jbe ffffffff81529bfc <__sanitizer_cov_trace_pc+0x5c> ffffffff81529bf5: 48 89 34 c1 mov %rsi,(%rcx,%rax,8) ffffffff81529bf9: 48 89 01 mov %rax,(%rcx) ffffffff81529bfc: c3 retq Oh, wait gcc-9 indeed does not inline: 0000000000000070 <__sanitizer_cov_trace_pc>: 70: 65 48 8b 0c 25 00 00 mov %gs:0x0,%rcx 77: 00 00 79: bf 02 00 00 00 mov $0x2,%edi 7e: 48 89 ce mov %rcx,%rsi 81: 4c 8b 04 24 mov (%rsp),%r8 85: e8 76 ff ff ff callq 0 <check_kcov_mode> 8a: 84 c0 test %al,%al 8c: 74 20 je ae <__sanitizer_cov_trace_pc+0x3e> 8e: 48 8b 91 b8 14 00 00 mov 0x14b8(%rcx),%rdx 95: 8b 89 b4 14 00 00 mov 0x14b4(%rcx),%ecx 9b: 48 8b 02 mov (%rdx),%rax 9e: 48 83 c0 01 add $0x1,%rax a2: 48 39 c1 cmp %rax,%rcx a5: 76 07 jbe ae <__sanitizer_cov_trace_pc+0x3e> a7: 4c 89 04 c2 mov %r8,(%rdx,%rax,8) ab: 48 89 02 mov %rax,(%rdx) ae: c3 retq This looks like a bug in gcc-8/9. gcc-6 inlines again as well as clang-11/12 inline. Please add __always_inline for check_kcov_mode. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE. 2021-03-18 7:17 ` Dmitry Vyukov @ 2021-03-21 18:43 ` Alexander Lochmann 2021-03-22 12:17 ` Miguel Ojeda 2021-03-23 7:23 ` Dmitry Vyukov 0 siblings, 2 replies; 13+ messages in thread From: Alexander Lochmann @ 2021-03-21 18:43 UTC (permalink / raw) Cc: Alexander Lochmann, Dmitry Vyukov, Andrey Konovalov, Jonathan Corbet, Miguel Ojeda, Randy Dunlap, Andrew Klychkov, Greg Kroah-Hartman, Andrew Morton, Aleksandr Nogikh, Jakub Kicinski, Wei Yongjun, Maciej Grochowski, kasan-dev, linux-doc, linux-kernel It simply stores the executed PCs. The execution order is discarded. Each bit in the shared buffer represents every fourth byte of the text segment. Since a call instruction on every supported architecture is at least four bytes, it is safe to just store every fourth byte of the text segment. In contrast to KCOV_MODE_TRACE_PC, the shared buffer cannot overflow. Thus, all executed PCs are recorded. Signed-off-by: Alexander Lochmann <info@alexander-lochmann.de> --- Documentation/dev-tools/kcov.rst | 80 +++++++++++++++++++++++++++ include/linux/kcov.h | 12 ++-- include/uapi/linux/kcov.h | 10 ++++ kernel/kcov.c | 94 ++++++++++++++++++++++++-------- 4 files changed, 169 insertions(+), 27 deletions(-) diff --git a/Documentation/dev-tools/kcov.rst b/Documentation/dev-tools/kcov.rst index d2c4c27e1702..e105ffe6b6e3 100644 --- a/Documentation/dev-tools/kcov.rst +++ b/Documentation/dev-tools/kcov.rst @@ -127,6 +127,86 @@ That is, a parent process opens /sys/kernel/debug/kcov, enables trace mode, mmaps coverage buffer and then forks child processes in a loop. Child processes only need to enable coverage (disable happens automatically on thread end). +If someone is interested in a set of executed PCs, and does not care about +execution order, he or she can advise KCOV to do so: + +.. code-block:: c + + #include <stdio.h> + #include <stddef.h> + #include <stdint.h> + #include <stdlib.h> + #include <sys/types.h> + #include <sys/stat.h> + #include <sys/ioctl.h> + #include <sys/mman.h> + #include <unistd.h> + #include <fcntl.h> + + #define KCOV_INIT_TRACE _IOR('c', 1, unsigned long) + #define KCOV_INIT_UNIQUE _IOR('c', 2, unsigned long) + #define KCOV_ENABLE _IO('c', 100) + #define KCOV_DISABLE _IO('c', 101) + + #define BITS_PER_LONG 64 + #define KCOV_TRACE_PC 0 + #define KCOV_TRACE_CMP 1 + #define KCOV_UNIQUE_PC 2 + /* + * Determine start of text segment via 'nm vmlinux | grep _stext | cut -d " " -f1', + * and fill in. + */ + #define STEXT_START 0xffffffff81000000 + + + + int main(int argc, char **argv) + { + int fd; + unsigned long *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"), exit(1); + /* Setup trace mode and trace size. */ + if ((n = ioctl(fd, KCOV_INIT_UNIQUE, 0)) < 0) + perror("ioctl"), exit(1); + /* Mmap buffer shared between kernel- and user-space. */ + cover = (unsigned long*)mmap(NULL, n, + PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); + if ((void*)cover == MAP_FAILED) + perror("mmap"), exit(1); + /* Enable coverage collection on the current thread. */ + if (ioctl(fd, KCOV_ENABLE, KCOV_UNIQUE_PC)) + perror("ioctl"), exit(1); + /* That's the target syscal call. */ + read(-1, NULL, 0); + /* 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"), exit(1); + /* Convert byte size into element size */ + n /= sizeof(unsigned long); + /* Print executed PCs in sorted order */ + for (i = 0; i < n; i++) { + for (int j = 0; j < BITS_PER_LONG; j++) { + if (cover[i] & (1L << j)) { + printf("0x%jx\n", (uintmax_t)(STEXT_START + (i * BITS_PER_LONG + j) * 4)); + } + } + } + /* Free resources. */ + if (munmap(cover, n * sizeof(unsigned long))) + perror("munmap"), exit(1); + if (close(fd)) + perror("close"), exit(1); + return 0; + } + Comparison operands collection ------------------------------ diff --git a/include/linux/kcov.h b/include/linux/kcov.h index 4e3037dc1204..d72dd73388d1 100644 --- a/include/linux/kcov.h +++ b/include/linux/kcov.h @@ -12,17 +12,21 @@ enum kcov_mode { /* Coverage collection is not enabled yet. */ KCOV_MODE_DISABLED = 0, /* KCOV was initialized, but tracing mode hasn't been chosen yet. */ - KCOV_MODE_INIT = 1, + KCOV_MODE_INIT_TRACE = 1, + /* KCOV was initialized, but recording of unique PCs hasn't been chosen yet. */ + KCOV_MODE_INIT_UNQIUE = 2, /* * Tracing coverage collection mode. * Covered PCs are collected in a per-task buffer. */ - KCOV_MODE_TRACE_PC = 2, + KCOV_MODE_TRACE_PC = 4, /* Collecting comparison operands mode. */ - KCOV_MODE_TRACE_CMP = 3, + KCOV_MODE_TRACE_CMP = 8, + /* Collecting unique covered PCs. Execution order is not saved. */ + KCOV_MODE_UNIQUE_PC = 16, }; -#define KCOV_IN_CTXSW (1 << 30) +#define KCOV_IN_CTXSW (1 << 31) void kcov_task_init(struct task_struct *t); void kcov_task_exit(struct task_struct *t); diff --git a/include/uapi/linux/kcov.h b/include/uapi/linux/kcov.h index 1d0350e44ae3..5b99b6d1a1ac 100644 --- a/include/uapi/linux/kcov.h +++ b/include/uapi/linux/kcov.h @@ -19,6 +19,7 @@ struct kcov_remote_arg { #define KCOV_REMOTE_MAX_HANDLES 0x100 #define KCOV_INIT_TRACE _IOR('c', 1, unsigned long) +#define KCOV_INIT_UNIQUE _IOR('c', 2, unsigned long) #define KCOV_ENABLE _IO('c', 100) #define KCOV_DISABLE _IO('c', 101) #define KCOV_REMOTE_ENABLE _IOW('c', 102, struct kcov_remote_arg) @@ -35,6 +36,15 @@ enum { KCOV_TRACE_PC = 0, /* Collecting comparison operands mode. */ KCOV_TRACE_CMP = 1, + /* + * Unique coverage collection mode. + * Unique covered PCs are collected in a per-task buffer. + * De-duplicates the collected PCs. Execution order is *not* saved. + * Each bit in the buffer represents every fourth byte of the text segment. + * Since a call instruction is at least four bytes on every supported + * architecture, storing just every fourth byte is sufficient. + */ + KCOV_UNIQUE_PC = 2, }; /* diff --git a/kernel/kcov.c b/kernel/kcov.c index 80bfe71bbe13..1f727043146a 100644 --- a/kernel/kcov.c +++ b/kernel/kcov.c @@ -24,6 +24,7 @@ #include <linux/refcount.h> #include <linux/log2.h> #include <asm/setup.h> +#include <asm/sections.h> #define kcov_debug(fmt, ...) pr_debug("%s: " fmt, __func__, ##__VA_ARGS__) @@ -151,10 +152,8 @@ static void kcov_remote_area_put(struct kcov_remote_area *area, list_add(&area->list, &kcov_remote_areas); } -static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t) +static __always_inline notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t, unsigned int *mode) { - unsigned int mode; - /* * We are interested in code coverage as a function of a syscall inputs, * so we ignore code executed in interrupts, unless we are in a remote @@ -162,7 +161,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_stru */ if (!in_task() && !(in_serving_softirq() && t->kcov_softirq)) return false; - mode = READ_ONCE(t->kcov_mode); + *mode = READ_ONCE(t->kcov_mode); /* * There is some code that runs in interrupts but for which * in_interrupt() returns false (e.g. preempt_schedule_irq()). @@ -171,7 +170,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_stru * kcov_start(). */ barrier(); - return mode == needed_mode; + return ((int)(*mode & (KCOV_IN_CTXSW | needed_mode))) > 0; } static notrace unsigned long canonicalize_ip(unsigned long ip) @@ -191,18 +190,27 @@ void notrace __sanitizer_cov_trace_pc(void) struct task_struct *t; unsigned long *area; unsigned long ip = canonicalize_ip(_RET_IP_); - unsigned long pos; + unsigned long pos, idx; + unsigned int mode; t = current; - if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t)) + if (!check_kcov_mode(KCOV_MODE_TRACE_PC | KCOV_MODE_UNIQUE_PC, t, &mode)) return; area = t->kcov_area; - /* The first 64-bit word is the number of subsequent PCs. */ - pos = READ_ONCE(area[0]) + 1; - if (likely(pos < t->kcov_size)) { - area[pos] = ip; - WRITE_ONCE(area[0], pos); + if (likely(mode == KCOV_MODE_TRACE_PC)) { + /* The first 64-bit word is the number of subsequent PCs. */ + pos = READ_ONCE(area[0]) + 1; + if (likely(pos < t->kcov_size)) { + area[pos] = ip; + WRITE_ONCE(area[0], pos); + } + } else { + idx = (ip - canonicalize_ip((unsigned long)&_stext)) / 4; + pos = idx % BITS_PER_LONG; + idx /= BITS_PER_LONG; + if (likely(idx < t->kcov_size)) + WRITE_ONCE(area[idx], READ_ONCE(area[idx]) | 1L << pos); } } EXPORT_SYMBOL(__sanitizer_cov_trace_pc); @@ -213,9 +221,10 @@ static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip) struct task_struct *t; u64 *area; u64 count, start_index, end_pos, max_pos; + unsigned int mode; t = current; - if (!check_kcov_mode(KCOV_MODE_TRACE_CMP, t)) + if (!check_kcov_mode(KCOV_MODE_TRACE_CMP, t, &mode)) return; ip = canonicalize_ip(ip); @@ -362,7 +371,7 @@ void kcov_task_init(struct task_struct *t) static void kcov_reset(struct kcov *kcov) { kcov->t = NULL; - kcov->mode = KCOV_MODE_INIT; + kcov->mode = KCOV_MODE_INIT_TRACE; kcov->remote = false; kcov->remote_size = 0; kcov->sequence++; @@ -468,12 +477,13 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma) spin_lock_irqsave(&kcov->lock, flags); size = kcov->size * sizeof(unsigned long); - if (kcov->mode != KCOV_MODE_INIT || vma->vm_pgoff != 0 || + if (kcov->mode & ~(KCOV_INIT_TRACE | KCOV_INIT_UNIQUE) || vma->vm_pgoff != 0 || vma->vm_end - vma->vm_start != size) { res = -EINVAL; goto exit; } if (!kcov->area) { + kcov_debug("mmap(): Allocating 0x%lx bytes\n", size); kcov->area = area; vma->vm_flags |= VM_DONTEXPAND; spin_unlock_irqrestore(&kcov->lock, flags); @@ -515,6 +525,8 @@ static int kcov_get_mode(unsigned long arg) { if (arg == KCOV_TRACE_PC) return KCOV_MODE_TRACE_PC; + else if (arg == KCOV_UNIQUE_PC) + return KCOV_MODE_UNIQUE_PC; else if (arg == KCOV_TRACE_CMP) #ifdef CONFIG_KCOV_ENABLE_COMPARISONS return KCOV_MODE_TRACE_CMP; @@ -562,12 +574,14 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, { struct task_struct *t; unsigned long size, unused; - int mode, i; + int mode, i, text_size, ret = 0; struct kcov_remote_arg *remote_arg; struct kcov_remote *remote; unsigned long flags; switch (cmd) { + case KCOV_INIT_UNIQUE: + /* fallthrough here */ case KCOV_INIT_TRACE: /* * Enable kcov in trace mode and setup buffer size. @@ -581,11 +595,41 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, * that must not overflow. */ size = arg; - if (size < 2 || size > INT_MAX / sizeof(unsigned long)) - return -EINVAL; - kcov->size = size; - kcov->mode = KCOV_MODE_INIT; - return 0; + if (cmd == KCOV_INIT_UNIQUE) { + if (size != 0) + return -EINVAL; + text_size = (canonicalize_ip((unsigned long)&_etext) - canonicalize_ip((unsigned long)&_stext)); + /** + * A call instr is at least four bytes on every supported architecture. + * Hence, just every fourth instruction can potentially be a call. + */ + text_size = roundup(text_size, 4); + text_size /= 4; + /* + * Round up size of text segment to multiple of BITS_PER_LONG. + * Otherwise, we cannot track + * the last (text_size % BITS_PER_LONG) addresses. + */ + text_size = roundup(text_size, BITS_PER_LONG); + /* Get the amount of bytes needed */ + text_size = text_size / 8; + /* mmap() requires size to be a multiple of PAGE_SIZE */ + text_size = roundup(text_size, PAGE_SIZE); + /* Get the cover size (= amount of bytes stored) */ + ret = text_size; + kcov->size = text_size / sizeof(unsigned long); + kcov_debug("text size = 0x%lx, roundup = 0x%x, kcov->size = 0x%x\n", + ((unsigned long)&_etext) - ((unsigned long)&_stext), + text_size, + kcov->size); + kcov->mode = KCOV_INIT_UNIQUE; + } else { + if (size < 2 || size > INT_MAX / sizeof(unsigned long)) + return -EINVAL; + kcov->size = size; + kcov->mode = KCOV_INIT_TRACE; + } + return ret; case KCOV_ENABLE: /* * Enable coverage for the current task. @@ -594,7 +638,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, * at task exit or voluntary by KCOV_DISABLE. After that it can * be enabled for another task. */ - if (kcov->mode != KCOV_MODE_INIT || !kcov->area) + if (!kcov->area) return -EINVAL; t = current; if (kcov->t != NULL || t->kcov != NULL) @@ -602,6 +646,10 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, mode = kcov_get_mode(arg); if (mode < 0) return mode; + if (kcov->mode == KCOV_INIT_TRACE && mode == KCOV_MODE_UNIQUE_PC) + return -EINVAL; + if (kcov->mode == KCOV_INIT_UNIQUE && (mode & (KCOV_MODE_TRACE_PC | KCOV_MODE_TRACE_CMP))) + return -EINVAL; kcov_fault_in_area(kcov); kcov->mode = mode; kcov_start(t, kcov, kcov->size, kcov->area, kcov->mode, @@ -622,7 +670,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, kcov_put(kcov); return 0; case KCOV_REMOTE_ENABLE: - if (kcov->mode != KCOV_MODE_INIT || !kcov->area) + if (kcov->mode != KCOV_MODE_INIT_TRACE || !kcov->area) return -EINVAL; t = current; if (kcov->t != NULL || t->kcov != NULL) -- 2.30.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE. 2021-03-21 18:43 ` [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE Alexander Lochmann @ 2021-03-22 12:17 ` Miguel Ojeda 2021-03-22 21:14 ` Alexander Lochmann 2021-03-23 7:23 ` Dmitry Vyukov 1 sibling, 1 reply; 13+ messages in thread From: Miguel Ojeda @ 2021-03-22 12:17 UTC (permalink / raw) To: Alexander Lochmann Cc: Dmitry Vyukov, Andrey Konovalov, Jonathan Corbet, Miguel Ojeda, Randy Dunlap, Andrew Klychkov, Greg Kroah-Hartman, Andrew Morton, Aleksandr Nogikh, Jakub Kicinski, Wei Yongjun, Maciej Grochowski, kasan-dev, Linux Doc Mailing List, linux-kernel Hi Alexander, On Sun, Mar 21, 2021 at 8:14 PM Alexander Lochmann <info@alexander-lochmann.de> wrote: > > diff --git a/Documentation/dev-tools/kcov.rst b/Documentation/dev-tools/kcov.rst > index d2c4c27e1702..e105ffe6b6e3 100644 > --- a/Documentation/dev-tools/kcov.rst > +++ b/Documentation/dev-tools/kcov.rst > @@ -127,6 +127,86 @@ That is, a parent process opens /sys/kernel/debug/kcov, enables trace mode, > mmaps coverage buffer and then forks child processes in a loop. Child processes > only need to enable coverage (disable happens automatically on thread end). > > +If someone is interested in a set of executed PCs, and does not care about > +execution order, he or she can advise KCOV to do so: Please mention explicitly that KCOV_INIT_UNIQUE should be used for that, i.e. readers of the example shouldn't need to read every line to figure it out. > + #define KCOV_INIT_TRACE _IOR('c', 1, unsigned long) Trace is not used in the example. > + /* KCOV was initialized, but recording of unique PCs hasn't been chosen yet. */ > + KCOV_MODE_INIT_UNQIUE = 2, Typo? It isn't used? PS: not sure why I was Cc'd, but I hope that helps. Cheers, Miguel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE. 2021-03-22 12:17 ` Miguel Ojeda @ 2021-03-22 21:14 ` Alexander Lochmann 0 siblings, 0 replies; 13+ messages in thread From: Alexander Lochmann @ 2021-03-22 21:14 UTC (permalink / raw) To: Miguel Ojeda Cc: Dmitry Vyukov, Andrey Konovalov, Jonathan Corbet, Miguel Ojeda, Randy Dunlap, Andrew Klychkov, Greg Kroah-Hartman, Andrew Morton, Aleksandr Nogikh, Jakub Kicinski, Wei Yongjun, Maciej Grochowski, kasan-dev, Linux Doc Mailing List, linux-kernel On 22.03.21 13:17, Miguel Ojeda wrote: > Hi Alexander, > > On Sun, Mar 21, 2021 at 8:14 PM Alexander Lochmann > <info@alexander-lochmann.de> wrote: >> >> diff --git a/Documentation/dev-tools/kcov.rst b/Documentation/dev-tools/kcov.rst >> index d2c4c27e1702..e105ffe6b6e3 100644 >> --- a/Documentation/dev-tools/kcov.rst >> +++ b/Documentation/dev-tools/kcov.rst >> @@ -127,6 +127,86 @@ That is, a parent process opens /sys/kernel/debug/kcov, enables trace mode, >> mmaps coverage buffer and then forks child processes in a loop. Child processes >> only need to enable coverage (disable happens automatically on thread end). >> >> +If someone is interested in a set of executed PCs, and does not care about >> +execution order, he or she can advise KCOV to do so: > > Please mention explicitly that KCOV_INIT_UNIQUE should be used for > that, i.e. readers of the example shouldn't need to read every line to > figure it out. > >> + #define KCOV_INIT_TRACE _IOR('c', 1, unsigned long) > > Trace is not used in the example. > >> + /* KCOV was initialized, but recording of unique PCs hasn't been chosen yet. */ >> + KCOV_MODE_INIT_UNQIUE = 2, > > Typo? It isn't used? It is a typo. It should be used... > > PS: not sure why I was Cc'd, but I hope that helps. Thx for your feedback. get_maintainer.pl told me to include you in Cc. Cheers, Alex > > Cheers, > Miguel > -- Alexander Lochmann PGP key: 0xBC3EF6FD Heiliger Weg 72 phone: +49.231.28053964 D-44141 Dortmund mobile: +49.151.15738323 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE. 2021-03-21 18:43 ` [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE Alexander Lochmann 2021-03-22 12:17 ` Miguel Ojeda @ 2021-03-23 7:23 ` Dmitry Vyukov 2021-03-23 8:19 ` Alexander Lochmann 1 sibling, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2021-03-23 7:23 UTC (permalink / raw) To: Alexander Lochmann Cc: Andrey Konovalov, Jonathan Corbet, Miguel Ojeda, Randy Dunlap, Andrew Klychkov, Greg Kroah-Hartman, Andrew Morton, Aleksandr Nogikh, Jakub Kicinski, Wei Yongjun, Maciej Grochowski, kasan-dev, open list:DOCUMENTATION, LKML, Gustavo A. R. Silva On Sun, Mar 21, 2021 at 7:44 PM Alexander Lochmann <info@alexander-lochmann.de> wrote: > > It simply stores the executed PCs. > The execution order is discarded. > Each bit in the shared buffer represents every fourth > byte of the text segment. > Since a call instruction on every supported > architecture is at least four bytes, it is safe > to just store every fourth byte of the text segment. > In contrast to KCOV_MODE_TRACE_PC, the shared buffer > cannot overflow. Thus, all executed PCs are recorded. > > Signed-off-by: Alexander Lochmann <info@alexander-lochmann.de> > --- > Documentation/dev-tools/kcov.rst | 80 +++++++++++++++++++++++++++ > include/linux/kcov.h | 12 ++-- > include/uapi/linux/kcov.h | 10 ++++ > kernel/kcov.c | 94 ++++++++++++++++++++++++-------- > 4 files changed, 169 insertions(+), 27 deletions(-) > > diff --git a/Documentation/dev-tools/kcov.rst b/Documentation/dev-tools/kcov.rst > index d2c4c27e1702..e105ffe6b6e3 100644 > --- a/Documentation/dev-tools/kcov.rst > +++ b/Documentation/dev-tools/kcov.rst > @@ -127,6 +127,86 @@ That is, a parent process opens /sys/kernel/debug/kcov, enables trace mode, > mmaps coverage buffer and then forks child processes in a loop. Child processes > only need to enable coverage (disable happens automatically on thread end). > > +If someone is interested in a set of executed PCs, and does not care about > +execution order, he or she can advise KCOV to do so: > + > +.. code-block:: c > + > + #include <stdio.h> > + #include <stddef.h> > + #include <stdint.h> > + #include <stdlib.h> > + #include <sys/types.h> > + #include <sys/stat.h> > + #include <sys/ioctl.h> > + #include <sys/mman.h> > + #include <unistd.h> > + #include <fcntl.h> > + > + #define KCOV_INIT_TRACE _IOR('c', 1, unsigned long) > + #define KCOV_INIT_UNIQUE _IOR('c', 2, unsigned long) > + #define KCOV_ENABLE _IO('c', 100) > + #define KCOV_DISABLE _IO('c', 101) > + > + #define BITS_PER_LONG 64 > + #define KCOV_TRACE_PC 0 > + #define KCOV_TRACE_CMP 1 > + #define KCOV_UNIQUE_PC 2 > + /* > + * Determine start of text segment via 'nm vmlinux | grep _stext | cut -d " " -f1', > + * and fill in. > + */ > + #define STEXT_START 0xffffffff81000000 > + > + > + > + int main(int argc, char **argv) > + { > + int fd; > + unsigned long *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"), exit(1); > + /* Setup trace mode and trace size. */ > + if ((n = ioctl(fd, KCOV_INIT_UNIQUE, 0)) < 0) > + perror("ioctl"), exit(1); > + /* Mmap buffer shared between kernel- and user-space. */ > + cover = (unsigned long*)mmap(NULL, n, > + PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); > + if ((void*)cover == MAP_FAILED) > + perror("mmap"), exit(1); > + /* Enable coverage collection on the current thread. */ > + if (ioctl(fd, KCOV_ENABLE, KCOV_UNIQUE_PC)) > + perror("ioctl"), exit(1); > + /* That's the target syscal call. */ > + read(-1, NULL, 0); > + /* 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"), exit(1); > + /* Convert byte size into element size */ > + n /= sizeof(unsigned long); > + /* Print executed PCs in sorted order */ > + for (i = 0; i < n; i++) { > + for (int j = 0; j < BITS_PER_LONG; j++) { > + if (cover[i] & (1L << j)) { > + printf("0x%jx\n", (uintmax_t)(STEXT_START + (i * BITS_PER_LONG + j) * 4)); > + } > + } > + } > + /* Free resources. */ > + if (munmap(cover, n * sizeof(unsigned long))) > + perror("munmap"), exit(1); > + if (close(fd)) > + perror("close"), exit(1); > + return 0; > + } > + > Comparison operands collection > ------------------------------ > > diff --git a/include/linux/kcov.h b/include/linux/kcov.h > index 4e3037dc1204..d72dd73388d1 100644 > --- a/include/linux/kcov.h > +++ b/include/linux/kcov.h > @@ -12,17 +12,21 @@ enum kcov_mode { > /* Coverage collection is not enabled yet. */ > KCOV_MODE_DISABLED = 0, > /* KCOV was initialized, but tracing mode hasn't been chosen yet. */ > - KCOV_MODE_INIT = 1, > + KCOV_MODE_INIT_TRACE = 1, > + /* KCOV was initialized, but recording of unique PCs hasn't been chosen yet. */ > + KCOV_MODE_INIT_UNQIUE = 2, > /* > * Tracing coverage collection mode. > * Covered PCs are collected in a per-task buffer. > */ > - KCOV_MODE_TRACE_PC = 2, > + KCOV_MODE_TRACE_PC = 4, > /* Collecting comparison operands mode. */ > - KCOV_MODE_TRACE_CMP = 3, > + KCOV_MODE_TRACE_CMP = 8, > + /* Collecting unique covered PCs. Execution order is not saved. */ > + KCOV_MODE_UNIQUE_PC = 16, > }; > > -#define KCOV_IN_CTXSW (1 << 30) > +#define KCOV_IN_CTXSW (1 << 31) > > void kcov_task_init(struct task_struct *t); > void kcov_task_exit(struct task_struct *t); > diff --git a/include/uapi/linux/kcov.h b/include/uapi/linux/kcov.h > index 1d0350e44ae3..5b99b6d1a1ac 100644 > --- a/include/uapi/linux/kcov.h > +++ b/include/uapi/linux/kcov.h > @@ -19,6 +19,7 @@ struct kcov_remote_arg { > #define KCOV_REMOTE_MAX_HANDLES 0x100 > > #define KCOV_INIT_TRACE _IOR('c', 1, unsigned long) > +#define KCOV_INIT_UNIQUE _IOR('c', 2, unsigned long) > #define KCOV_ENABLE _IO('c', 100) > #define KCOV_DISABLE _IO('c', 101) > #define KCOV_REMOTE_ENABLE _IOW('c', 102, struct kcov_remote_arg) > @@ -35,6 +36,15 @@ enum { > KCOV_TRACE_PC = 0, > /* Collecting comparison operands mode. */ > KCOV_TRACE_CMP = 1, > + /* > + * Unique coverage collection mode. > + * Unique covered PCs are collected in a per-task buffer. > + * De-duplicates the collected PCs. Execution order is *not* saved. > + * Each bit in the buffer represents every fourth byte of the text segment. > + * Since a call instruction is at least four bytes on every supported > + * architecture, storing just every fourth byte is sufficient. > + */ > + KCOV_UNIQUE_PC = 2, > }; > > /* > diff --git a/kernel/kcov.c b/kernel/kcov.c > index 80bfe71bbe13..1f727043146a 100644 > --- a/kernel/kcov.c > +++ b/kernel/kcov.c > @@ -24,6 +24,7 @@ > #include <linux/refcount.h> > #include <linux/log2.h> > #include <asm/setup.h> > +#include <asm/sections.h> Is this for __always_inline? __always_inline is defined in include/linux/compiler_types.h. > > #define kcov_debug(fmt, ...) pr_debug("%s: " fmt, __func__, ##__VA_ARGS__) > > @@ -151,10 +152,8 @@ static void kcov_remote_area_put(struct kcov_remote_area *area, > list_add(&area->list, &kcov_remote_areas); > } > > -static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t) > +static __always_inline notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t, unsigned int *mode) > { > - unsigned int mode; > - > /* > * We are interested in code coverage as a function of a syscall inputs, > * so we ignore code executed in interrupts, unless we are in a remote > @@ -162,7 +161,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_stru > */ > if (!in_task() && !(in_serving_softirq() && t->kcov_softirq)) > return false; > - mode = READ_ONCE(t->kcov_mode); > + *mode = READ_ONCE(t->kcov_mode); > /* > * There is some code that runs in interrupts but for which > * in_interrupt() returns false (e.g. preempt_schedule_irq()). > @@ -171,7 +170,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_stru > * kcov_start(). > */ > barrier(); > - return mode == needed_mode; > + return ((int)(*mode & (KCOV_IN_CTXSW | needed_mode))) > 0; This logic and the rest of the patch looks good to me. Thanks > } > > static notrace unsigned long canonicalize_ip(unsigned long ip) > @@ -191,18 +190,27 @@ void notrace __sanitizer_cov_trace_pc(void) > struct task_struct *t; > unsigned long *area; > unsigned long ip = canonicalize_ip(_RET_IP_); > - unsigned long pos; > + unsigned long pos, idx; > + unsigned int mode; > > t = current; > - if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t)) > + if (!check_kcov_mode(KCOV_MODE_TRACE_PC | KCOV_MODE_UNIQUE_PC, t, &mode)) > return; > > area = t->kcov_area; > - /* The first 64-bit word is the number of subsequent PCs. */ > - pos = READ_ONCE(area[0]) + 1; > - if (likely(pos < t->kcov_size)) { > - area[pos] = ip; > - WRITE_ONCE(area[0], pos); > + if (likely(mode == KCOV_MODE_TRACE_PC)) { > + /* The first 64-bit word is the number of subsequent PCs. */ > + pos = READ_ONCE(area[0]) + 1; > + if (likely(pos < t->kcov_size)) { > + area[pos] = ip; > + WRITE_ONCE(area[0], pos); > + } > + } else { > + idx = (ip - canonicalize_ip((unsigned long)&_stext)) / 4; > + pos = idx % BITS_PER_LONG; > + idx /= BITS_PER_LONG; > + if (likely(idx < t->kcov_size)) > + WRITE_ONCE(area[idx], READ_ONCE(area[idx]) | 1L << pos); > } > } > EXPORT_SYMBOL(__sanitizer_cov_trace_pc); > @@ -213,9 +221,10 @@ static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip) > struct task_struct *t; > u64 *area; > u64 count, start_index, end_pos, max_pos; > + unsigned int mode; > > t = current; > - if (!check_kcov_mode(KCOV_MODE_TRACE_CMP, t)) > + if (!check_kcov_mode(KCOV_MODE_TRACE_CMP, t, &mode)) > return; > > ip = canonicalize_ip(ip); > @@ -362,7 +371,7 @@ void kcov_task_init(struct task_struct *t) > static void kcov_reset(struct kcov *kcov) > { > kcov->t = NULL; > - kcov->mode = KCOV_MODE_INIT; > + kcov->mode = KCOV_MODE_INIT_TRACE; > kcov->remote = false; > kcov->remote_size = 0; > kcov->sequence++; > @@ -468,12 +477,13 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma) > > spin_lock_irqsave(&kcov->lock, flags); > size = kcov->size * sizeof(unsigned long); > - if (kcov->mode != KCOV_MODE_INIT || vma->vm_pgoff != 0 || > + if (kcov->mode & ~(KCOV_INIT_TRACE | KCOV_INIT_UNIQUE) || vma->vm_pgoff != 0 || > vma->vm_end - vma->vm_start != size) { > res = -EINVAL; > goto exit; > } > if (!kcov->area) { > + kcov_debug("mmap(): Allocating 0x%lx bytes\n", size); > kcov->area = area; > vma->vm_flags |= VM_DONTEXPAND; > spin_unlock_irqrestore(&kcov->lock, flags); > @@ -515,6 +525,8 @@ static int kcov_get_mode(unsigned long arg) > { > if (arg == KCOV_TRACE_PC) > return KCOV_MODE_TRACE_PC; > + else if (arg == KCOV_UNIQUE_PC) > + return KCOV_MODE_UNIQUE_PC; > else if (arg == KCOV_TRACE_CMP) > #ifdef CONFIG_KCOV_ENABLE_COMPARISONS > return KCOV_MODE_TRACE_CMP; > @@ -562,12 +574,14 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, > { > struct task_struct *t; > unsigned long size, unused; > - int mode, i; > + int mode, i, text_size, ret = 0; > struct kcov_remote_arg *remote_arg; > struct kcov_remote *remote; > unsigned long flags; > > switch (cmd) { > + case KCOV_INIT_UNIQUE: > + /* fallthrough here */ Looking at "git log --grep fallthrough", it seems that the modern way to say this is to use the fallthrough keyword. Please run checkpatch, it shows a bunch of other warnings as well: git diff HEAD^ | scripts/checkpatch.pl - > case KCOV_INIT_TRACE: > /* > * Enable kcov in trace mode and setup buffer size. > @@ -581,11 +595,41 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, > * that must not overflow. > */ > size = arg; > - if (size < 2 || size > INT_MAX / sizeof(unsigned long)) > - return -EINVAL; > - kcov->size = size; > - kcov->mode = KCOV_MODE_INIT; > - return 0; > + if (cmd == KCOV_INIT_UNIQUE) { > + if (size != 0) > + return -EINVAL; > + text_size = (canonicalize_ip((unsigned long)&_etext) - canonicalize_ip((unsigned long)&_stext)); > + /** > + * A call instr is at least four bytes on every supported architecture. > + * Hence, just every fourth instruction can potentially be a call. > + */ > + text_size = roundup(text_size, 4); > + text_size /= 4; > + /* > + * Round up size of text segment to multiple of BITS_PER_LONG. > + * Otherwise, we cannot track > + * the last (text_size % BITS_PER_LONG) addresses. > + */ > + text_size = roundup(text_size, BITS_PER_LONG); > + /* Get the amount of bytes needed */ > + text_size = text_size / 8; > + /* mmap() requires size to be a multiple of PAGE_SIZE */ > + text_size = roundup(text_size, PAGE_SIZE); > + /* Get the cover size (= amount of bytes stored) */ > + ret = text_size; > + kcov->size = text_size / sizeof(unsigned long); > + kcov_debug("text size = 0x%lx, roundup = 0x%x, kcov->size = 0x%x\n", > + ((unsigned long)&_etext) - ((unsigned long)&_stext), > + text_size, > + kcov->size); > + kcov->mode = KCOV_INIT_UNIQUE; > + } else { > + if (size < 2 || size > INT_MAX / sizeof(unsigned long)) > + return -EINVAL; > + kcov->size = size; > + kcov->mode = KCOV_INIT_TRACE; > + } > + return ret; > case KCOV_ENABLE: > /* > * Enable coverage for the current task. > @@ -594,7 +638,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, > * at task exit or voluntary by KCOV_DISABLE. After that it can > * be enabled for another task. > */ > - if (kcov->mode != KCOV_MODE_INIT || !kcov->area) > + if (!kcov->area) > return -EINVAL; > t = current; > if (kcov->t != NULL || t->kcov != NULL) > @@ -602,6 +646,10 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, > mode = kcov_get_mode(arg); > if (mode < 0) > return mode; > + if (kcov->mode == KCOV_INIT_TRACE && mode == KCOV_MODE_UNIQUE_PC) > + return -EINVAL; > + if (kcov->mode == KCOV_INIT_UNIQUE && (mode & (KCOV_MODE_TRACE_PC | KCOV_MODE_TRACE_CMP))) > + return -EINVAL; > kcov_fault_in_area(kcov); > kcov->mode = mode; > kcov_start(t, kcov, kcov->size, kcov->area, kcov->mode, > @@ -622,7 +670,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, > kcov_put(kcov); > return 0; > case KCOV_REMOTE_ENABLE: > - if (kcov->mode != KCOV_MODE_INIT || !kcov->area) > + if (kcov->mode != KCOV_MODE_INIT_TRACE || !kcov->area) > return -EINVAL; > t = current; > if (kcov->t != NULL || t->kcov != NULL) > -- > 2.30.2 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE. 2021-03-23 7:23 ` Dmitry Vyukov @ 2021-03-23 8:19 ` Alexander Lochmann 0 siblings, 0 replies; 13+ messages in thread From: Alexander Lochmann @ 2021-03-23 8:19 UTC (permalink / raw) To: Dmitry Vyukov Cc: Andrey Konovalov, Jonathan Corbet, Miguel Ojeda, Randy Dunlap, Andrew Klychkov, Greg Kroah-Hartman, Andrew Morton, Aleksandr Nogikh, Jakub Kicinski, Wei Yongjun, Maciej Grochowski, kasan-dev, open list:DOCUMENTATION, LKML, Gustavo A. R. Silva On 23.03.21 08:23, Dmitry Vyukov wrote: >> diff --git a/kernel/kcov.c b/kernel/kcov.c >> index 80bfe71bbe13..1f727043146a 100644 >> --- a/kernel/kcov.c >> +++ b/kernel/kcov.c >> @@ -24,6 +24,7 @@ >> #include <linux/refcount.h> >> #include <linux/log2.h> >> #include <asm/setup.h> >> +#include <asm/sections.h> > > Is this for __always_inline? > __always_inline is defined in include/linux/compiler_types.h. > This is for the symbols marking start and end of the text segment (_stext/_etext). > >> >> #define kcov_debug(fmt, ...) pr_debug("%s: " fmt, __func__, ##__VA_ARGS__) >> >> @@ -151,10 +152,8 @@ static void kcov_remote_area_put(struct kcov_remote_area *area, >> list_add(&area->list, &kcov_remote_areas); >> } >> >> -static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t) >> +static __always_inline notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_struct *t, unsigned int *mode) >> { >> - unsigned int mode; >> - >> /* >> * We are interested in code coverage as a function of a syscall inputs, >> * so we ignore code executed in interrupts, unless we are in a remote >> @@ -162,7 +161,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_stru >> */ >> if (!in_task() && !(in_serving_softirq() && t->kcov_softirq)) >> return false; >> - mode = READ_ONCE(t->kcov_mode); >> + *mode = READ_ONCE(t->kcov_mode); >> /* >> * There is some code that runs in interrupts but for which >> * in_interrupt() returns false (e.g. preempt_schedule_irq()). >> @@ -171,7 +170,7 @@ static notrace bool check_kcov_mode(enum kcov_mode needed_mode, struct task_stru >> * kcov_start(). >> */ >> barrier(); >> - return mode == needed_mode; >> + return ((int)(*mode & (KCOV_IN_CTXSW | needed_mode))) > 0; > > This logic and the rest of the patch looks good to me. > > Thanks Thx. > >> } >> >> static notrace unsigned long canonicalize_ip(unsigned long ip) >> @@ -191,18 +190,27 @@ void notrace __sanitizer_cov_trace_pc(void) >> struct task_struct *t; >> unsigned long *area; >> unsigned long ip = canonicalize_ip(_RET_IP_); >> - unsigned long pos; >> + unsigned long pos, idx; >> + unsigned int mode; >> >> t = current; >> - if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t)) >> + if (!check_kcov_mode(KCOV_MODE_TRACE_PC | KCOV_MODE_UNIQUE_PC, t, &mode)) >> return; >> >> area = t->kcov_area; >> - /* The first 64-bit word is the number of subsequent PCs. */ >> - pos = READ_ONCE(area[0]) + 1; >> - if (likely(pos < t->kcov_size)) { >> - area[pos] = ip; >> - WRITE_ONCE(area[0], pos); >> + if (likely(mode == KCOV_MODE_TRACE_PC)) { >> + /* The first 64-bit word is the number of subsequent PCs. */ >> + pos = READ_ONCE(area[0]) + 1; >> + if (likely(pos < t->kcov_size)) { >> + area[pos] = ip; >> + WRITE_ONCE(area[0], pos); >> + } >> + } else { >> + idx = (ip - canonicalize_ip((unsigned long)&_stext)) / 4; >> + pos = idx % BITS_PER_LONG; >> + idx /= BITS_PER_LONG; >> + if (likely(idx < t->kcov_size)) >> + WRITE_ONCE(area[idx], READ_ONCE(area[idx]) | 1L << pos); >> } >> } >> EXPORT_SYMBOL(__sanitizer_cov_trace_pc); >> @@ -213,9 +221,10 @@ static void notrace write_comp_data(u64 type, u64 arg1, u64 arg2, u64 ip) >> struct task_struct *t; >> u64 *area; >> u64 count, start_index, end_pos, max_pos; >> + unsigned int mode; >> >> t = current; >> - if (!check_kcov_mode(KCOV_MODE_TRACE_CMP, t)) >> + if (!check_kcov_mode(KCOV_MODE_TRACE_CMP, t, &mode)) >> return; >> >> ip = canonicalize_ip(ip); >> @@ -362,7 +371,7 @@ void kcov_task_init(struct task_struct *t) >> static void kcov_reset(struct kcov *kcov) >> { >> kcov->t = NULL; >> - kcov->mode = KCOV_MODE_INIT; >> + kcov->mode = KCOV_MODE_INIT_TRACE; >> kcov->remote = false; >> kcov->remote_size = 0; >> kcov->sequence++; >> @@ -468,12 +477,13 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma) >> >> spin_lock_irqsave(&kcov->lock, flags); >> size = kcov->size * sizeof(unsigned long); >> - if (kcov->mode != KCOV_MODE_INIT || vma->vm_pgoff != 0 || >> + if (kcov->mode & ~(KCOV_INIT_TRACE | KCOV_INIT_UNIQUE) || vma->vm_pgoff != 0 || >> vma->vm_end - vma->vm_start != size) { >> res = -EINVAL; >> goto exit; >> } >> if (!kcov->area) { >> + kcov_debug("mmap(): Allocating 0x%lx bytes\n", size); >> kcov->area = area; >> vma->vm_flags |= VM_DONTEXPAND; >> spin_unlock_irqrestore(&kcov->lock, flags); >> @@ -515,6 +525,8 @@ static int kcov_get_mode(unsigned long arg) >> { >> if (arg == KCOV_TRACE_PC) >> return KCOV_MODE_TRACE_PC; >> + else if (arg == KCOV_UNIQUE_PC) >> + return KCOV_MODE_UNIQUE_PC; >> else if (arg == KCOV_TRACE_CMP) >> #ifdef CONFIG_KCOV_ENABLE_COMPARISONS >> return KCOV_MODE_TRACE_CMP; >> @@ -562,12 +574,14 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, >> { >> struct task_struct *t; >> unsigned long size, unused; >> - int mode, i; >> + int mode, i, text_size, ret = 0; >> struct kcov_remote_arg *remote_arg; >> struct kcov_remote *remote; >> unsigned long flags; >> >> switch (cmd) { >> + case KCOV_INIT_UNIQUE: >> + /* fallthrough here */ > > Looking at "git log --grep fallthrough", it seems that the modern way > to say this is to use the fallthrough keyword. > > Please run checkpatch, it shows a bunch of other warnings as well: > > git diff HEAD^ | scripts/checkpatch.pl - Yeah. I'll do that. -- Alexander Lochmann PGP key: 0xBC3EF6FD Heiliger Weg 72 phone: +49.231.28053964 D-44141 Dortmund mobile: +49.151.15738323 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-03-23 8:29 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-11 8:07 [PATCH] KCOV: Introduced tracing unique covered PCs Alexander Lochmann 2021-02-12 12:54 ` Dmitry Vyukov 2021-03-14 21:29 ` Alexander Lochmann 2021-03-15 8:02 ` Dmitry Vyukov 2021-03-15 21:43 ` Alexander Lochmann 2021-03-16 6:36 ` Dmitry Vyukov 2021-03-17 21:10 ` Alexander Lochmann 2021-03-18 7:17 ` Dmitry Vyukov 2021-03-21 18:43 ` [PATCH] Introduced new tracing mode KCOV_MODE_UNIQUE Alexander Lochmann 2021-03-22 12:17 ` Miguel Ojeda 2021-03-22 21:14 ` Alexander Lochmann 2021-03-23 7:23 ` Dmitry Vyukov 2021-03-23 8:19 ` Alexander Lochmann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.