On Wed, Nov 16, 2016 at 10:27 PM, Quentin Casasnovas <quentin.casasno...@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 <dvyu...@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 <dvyu...@google.com> > Cc: Michal Zalewski <lcam...@gmail.com> > Cc: Kees Cook <keesc...@chromium.org> > Signed-off-by: Quentin Casasnovas <quentin.casasno...@oracle.com> > Signed-off-by: Vegard Nossum <vegard.nos...@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 >