Messages in this thread Patch in this message | | | From | "JianGen Jiao (QUIC)" <> | Subject | RE: [PATCH] kcov: add KCOV_PC_RANGE to limit pc range | Date | Fri, 19 Nov 2021 03:17:25 +0000 |
| |
Hi Dmitry, I'm using the start, end pc from cover filter, which currently is the fast way compared to the big bitmap passing from syzkaller solution, as I only set the cover filter to dirs/files I care about.
I checked https://groups.google.com/g/kasan-dev/c/oVz3ZSWaK1Q/m/9ASztdzCAAAJ, The bitmap seems not the same as syzkaller one, which one will be used finally?
``` Alexander's one + pos = (ip - canonicalize_ip((unsigned long)&_stext)) / 4; + idx = pos % BITS_PER_LONG; + pos /= BITS_PER_LONG; + if (likely(pos < t->kcov_size)) + WRITE_ONCE(area[pos], READ_ONCE(area[pos]) | 1L << idx); ``` Pc offset is divided by 4 and start is _stext. But for some arch, pc is less than _stext.
``` https://github.com/google/syzkaller/blob/master/syz-manager/covfilter.go#L139-L154 data := make([]byte, 8+((size>>4)/8+1)) order := binary.ByteOrder(binary.BigEndian) if target.LittleEndian { order = binary.LittleEndian } order.PutUint32(data, start) order.PutUint32(data[4:], size)
bitmap := data[8:] for pc := range pcs { // The lowest 4-bit is dropped. pc = uint32(backend.NextInstructionPC(target, uint64(pc))) pc = (pc - start) >> 4 bitmap[pc/8] |= (1 << (pc % 8)) } return data ``` Pc offset is divided by 16 and start is cover filter start pc.
I think divided by 8 is more reasonable? Because there is at least one instruction before each __sanitizer_cov_trace_pc call. 0000000000000160 R_AARCH64_CALL26 __sanitizer_cov_trace_pc 0000000000000168 R_AARCH64_CALL26 __sanitizer_cov_trace_pc
I think we still need my patch because we still need a way to keep the trace_pc call and post-filter in syzkaller doesn't solve trace_pc dropping, right? But for sure I can use the bitmap from syzkaller.
THX Joey -----Original Message----- From: Dmitry Vyukov <dvyukov@google.com> Sent: Thursday, November 18, 2021 10:00 PM To: JianGen Jiao (QUIC) <quic_jiangenj@quicinc.com> Cc: andreyknvl@gmail.com; kasan-dev@googlegroups.com; LKML <linux-kernel@vger.kernel.org>; Alexander Lochmann <info@alexander-lochmann.de> Subject: Re: [PATCH] kcov: add KCOV_PC_RANGE to limit pc range
WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
,On Wed, 17 Nov 2021 at 07:24, Joey Jiao <quic_jiangenj@quicinc.com> wrote: > > Sometimes we only interested in the pcs within some range, while there > are cases these pcs are dropped by kernel due to `pos >= > t->kcov_size`, and by increasing the map area size doesn't help. > > To avoid disabling KCOV for these not intereseted pcs during build > time, adding this new KCOV_PC_RANGE cmd.
Hi Joey,
How do you use this? I am concerned that a single range of PCs is too restrictive. I can only see how this can work for single module (continuous in memory) or a single function. But for anything else (something in the main kernel, or several modules), it won't work as PCs are not continuous.
Maybe we should use a compressed bitmap of interesting PCs? It allows to support all cases and we already have it in syz-executor, then syz-executor could simply pass the bitmap to the kernel rather than post-filter. It's also overlaps with the KCOV_MODE_UNIQUE mode that +Alexander proposed here: https://groups.google.com/g/kasan-dev/c/oVz3ZSWaK1Q/m/9ASztdzCAAAJ It would be reasonable if kernel uses the same bitmap format for these 2 features.
> An example usage is to use together syzkaller's cov filter. > > Change-Id: I954f6efe1bca604f5ce31f8f2b6f689e34a2981d > Signed-off-by: Joey Jiao <quic_jiangenj@quicinc.com> > --- > Documentation/dev-tools/kcov.rst | 10 ++++++++++ > include/uapi/linux/kcov.h | 7 +++++++ > kernel/kcov.c | 18 ++++++++++++++++++ > 3 files changed, 35 insertions(+) > > diff --git a/Documentation/dev-tools/kcov.rst > b/Documentation/dev-tools/kcov.rst > index d83c9ab..fbcd422 100644 > --- a/Documentation/dev-tools/kcov.rst > +++ b/Documentation/dev-tools/kcov.rst > @@ -52,9 +52,15 @@ program using kcov: > #include <fcntl.h> > #include <linux/types.h> > > + struct kcov_pc_range { > + uint32 start; > + uint32 end; > + }; > + > #define KCOV_INIT_TRACE _IOR('c', 1, unsigned long) > #define KCOV_ENABLE _IO('c', 100) > #define KCOV_DISABLE _IO('c', 101) > + #define KCOV_TRACE_RANGE _IOW('c', 103, struct kcov_pc_range) > #define COVER_SIZE (64<<10) > > #define KCOV_TRACE_PC 0 > @@ -64,6 +70,8 @@ program using kcov: > { > int fd; > unsigned long *cover, n, i; > + /* Change start and/or end to your interested pc range. */ > + struct kcov_pc_range pc_range = {.start = 0, .end = > + (uint32)(~((uint32)0))}; > > /* A single fd descriptor allows coverage collection on a single > * thread. > @@ -79,6 +87,8 @@ program using kcov: > PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); > if ((void*)cover == MAP_FAILED) > perror("mmap"), exit(1); > + if (ioctl(fd, KCOV_PC_RANGE, pc_range)) > + dprintf(2, "ignore KCOV_PC_RANGE error.\n"); > /* Enable coverage collection on the current thread. */ > if (ioctl(fd, KCOV_ENABLE, KCOV_TRACE_PC)) > perror("ioctl"), exit(1); diff --git > a/include/uapi/linux/kcov.h b/include/uapi/linux/kcov.h index > 1d0350e..353ff0a 100644 > --- a/include/uapi/linux/kcov.h > +++ b/include/uapi/linux/kcov.h > @@ -16,12 +16,19 @@ struct kcov_remote_arg { > __aligned_u64 handles[0]; > }; > > +#define PC_RANGE_MASK ((__u32)(~((u32) 0))) struct kcov_pc_range { > + __u32 start; /* start pc & 0xFFFFFFFF */ > + __u32 end; /* end pc & 0xFFFFFFFF */ > +}; > + > #define KCOV_REMOTE_MAX_HANDLES 0x100 > > #define KCOV_INIT_TRACE _IOR('c', 1, 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) > +#define KCOV_PC_RANGE _IOW('c', 103, struct kcov_pc_range) > > enum { > /* > diff --git a/kernel/kcov.c b/kernel/kcov.c index 36ca640..59550450 > 100644 > --- a/kernel/kcov.c > +++ b/kernel/kcov.c > @@ -36,6 +36,7 @@ > * - initial state after open() > * - then there must be a single ioctl(KCOV_INIT_TRACE) call > * - then, mmap() call (several calls are allowed but not useful) > + * - then, optional to set trace pc range > * - then, ioctl(KCOV_ENABLE, arg), where arg is > * KCOV_TRACE_PC - to trace only the PCs > * or > @@ -69,6 +70,8 @@ struct kcov { > * kcov_remote_stop(), see the comment there. > */ > int sequence; > + /* u32 Trace PC range from start to end. */ > + struct kcov_pc_range pc_range; > }; > > struct kcov_remote_area { > @@ -192,6 +195,7 @@ static notrace unsigned long > canonicalize_ip(unsigned long ip) void notrace > __sanitizer_cov_trace_pc(void) { > struct task_struct *t; > + struct kcov_pc_range pc_range; > unsigned long *area; > unsigned long ip = canonicalize_ip(_RET_IP_); > unsigned long pos; > @@ -199,6 +203,11 @@ void notrace __sanitizer_cov_trace_pc(void) > t = current; > if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t)) > return; > + pc_range = t->kcov->pc_range; > + if (pc_range.start < pc_range.end && > + ((ip & PC_RANGE_MASK) < pc_range.start || > + (ip & PC_RANGE_MASK) > pc_range.end)) > + return; > > area = t->kcov_area; > /* The first 64-bit word is the number of subsequent PCs. */ > @@ -568,6 +577,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, > int mode, i; > struct kcov_remote_arg *remote_arg; > struct kcov_remote *remote; > + struct kcov_pc_range *pc_range; > unsigned long flags; > > switch (cmd) { > @@ -589,6 +599,14 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, > kcov->size = size; > kcov->mode = KCOV_MODE_INIT; > return 0; > + case KCOV_PC_RANGE: > + /* Limit trace pc range. */ > + pc_range = (struct kcov_pc_range *)arg; > + if (copy_from_user(&kcov->pc_range, pc_range, sizeof(kcov->pc_range))) > + return -EINVAL; > + if (kcov->pc_range.start >= kcov->pc_range.end) > + return -EINVAL; > + return 0; > case KCOV_ENABLE: > /* > * Enable coverage for the current task. > -- > 2.7.4 >
| |