lkml.org 
[lkml]   [2016]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 1/2] kcov: size of arena is now given in bytes.
On Wed, Nov 16, 2016 at 10:27 PM, Quentin Casasnovas
<quentin.casasnovas@oracle.com> wrote:
> We'll introduce a different mode of tracing a-la AFL fixed table and Dmitry
> suggests that the code would be simpler with the size expressed in bytes as
> opposed unsigned longs.
>
> We only change the kcov::size field, which will be shared between different
> modes, but leave the task_struct::kcov_size field expressed in unsigned
> long in order to save an unecessary bitshift/division in the hot path when
> using KCOV_MODE_TRACE.


Thanks!

Reviewed-and-tested-by: Dmitry Vyukov <dvyukov@google.com>

I've tested that KCOV_MODE_TRACE works for me with these two patches applied.

> but leave the task_struct::kcov_size field expressed in unsigned long

The only purpose of the cache in task struct is to make coverage fast
path fast, so this looks good to me.



> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Michal Zalewski <lcamtuf@gmail.com>
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
> kernel/kcov.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index 30e6d05aa5a9..c2aa93851f93 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -32,7 +32,7 @@ struct kcov {
> /* The lock protects mode, size, area and t. */
> spinlock_t lock;
> enum kcov_mode mode;
> - /* Size of arena (in long's for KCOV_MODE_TRACE). */
> + /* Size of arena in bytes. */
> unsigned size;
> /* Coverage buffer shared with user space. */
> void *area;
> @@ -140,7 +140,7 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma)
> return -ENOMEM;
>
> spin_lock(&kcov->lock);
> - size = kcov->size * sizeof(unsigned long);
> + size = kcov->size;
> if (kcov->mode == KCOV_MODE_DISABLED || vma->vm_pgoff != 0 ||
> vma->vm_end - vma->vm_start != size) {
> res = -EINVAL;
> @@ -198,13 +198,11 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
> return -EBUSY;
> /*
> * Size must be at least 2 to hold current position and one PC.
> - * Later we allocate size * sizeof(unsigned long) memory,
> - * that must not overflow.
> */
> size = arg;
> if (size < 2 || size > INT_MAX / sizeof(unsigned long))
> return -EINVAL;
> - kcov->size = size;
> + kcov->size = size * sizeof(unsigned long);
> kcov->mode = KCOV_MODE_TRACE;
> return 0;
> case KCOV_ENABLE:
> @@ -223,7 +221,7 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd,
> return -EBUSY;
> t = current;
> /* Cache in task struct for performance. */
> - t->kcov_size = kcov->size;
> + t->kcov_size = kcov->size / sizeof(unsigned long);
> t->kcov_area = kcov->area;
> /* See comment in __sanitizer_cov_trace_pc(). */
> barrier();
> --
> 2.10.2
>

\
 
 \ /
  Last update: 2016-11-17 08:55    [W:0.069 / U:5.640 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site