| 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
| ||
|
Message-ID: <DM8PR02MB8247720860A08914CAA41D42F89C9@DM8PR02MB8247.namprd02.prod.outlook.com>
Date: Fri, 19 Nov 2021 03:17:25 +0000
From: "JianGen Jiao (QUIC)" <quic_jiangenj@...cinc.com>
To: Dmitry Vyukov <dvyukov@...gle.com>,
"JianGen Jiao (QUIC)" <quic_jiangenj@...cinc.com>
CC: "andreyknvl@...il.com" <andreyknvl@...il.com>,
"kasan-dev@...glegroups.com" <kasan-dev@...glegroups.com>,
LKML <linux-kernel@...r.kernel.org>,
Alexander Lochmann <info@...xander-lochmann.de>,
"Likai Ding (QUIC)" <quic_likaid@...cinc.com>
Subject: RE: [PATCH] kcov: add KCOV_PC_RANGE to limit pc range
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@...gle.com>
Sent: Thursday, November 18, 2021 10:00 PM
To: JianGen Jiao (QUIC) <quic_jiangenj@...cinc.com>
Cc: andreyknvl@...il.com; kasan-dev@...glegroups.com; LKML <linux-kernel@...r.kernel.org>; Alexander Lochmann <info@...xander-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@...cinc.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@...cinc.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
>
Powered by blists - more mailing lists