Messages in this thread | | | From | "Liang, Kan" <> | Subject | RE: [PATCH] perf/x86/uncore: remove event_list for snb client uncore IMC | Date | Wed, 16 Nov 2016 14:32:35 +0000 |
| |
> > * kan.liang@intel.com <kan.liang@intel.com> wrote: > > > From: Kan Liang <kan.liang@intel.com> > > > > A BUG was found by perf_fuzzer after enabled KASAN. > > [ 205.748005] BUG: KASAN: slab-out-of-bounds in > > snb_uncore_imc_event_del+0x6c/0xa0 at addr ffff8800caa43768 > > > Reported-by: Vince Weaver <vincent.weaver@maine.edu> > > Tested-by: Vince Weaver <vincent.weaver@maine.edu> > > Signed-off-by: Kan Liang <kan.liang@intel.com> > > --- > > arch/x86/events/intel/uncore_snb.c | 12 ------------ > > 1 file changed, 12 deletions(-) > > > > diff --git a/arch/x86/events/intel/uncore_snb.c > > b/arch/x86/events/intel/uncore_snb.c > > index 81195cc..a3dcc12 100644 > > --- a/arch/x86/events/intel/uncore_snb.c > > +++ b/arch/x86/events/intel/uncore_snb.c > > @@ -490,24 +490,12 @@ static int snb_uncore_imc_event_add(struct > > perf_event *event, int flags) > > > > snb_uncore_imc_event_start(event, 0); > > > > - box->n_events++; > > - > > return 0; > > } > > > > static void snb_uncore_imc_event_del(struct perf_event *event, int > > flags) { > > - struct intel_uncore_box *box = uncore_event_to_box(event); > > - int i; > > - > > snb_uncore_imc_event_stop(event, PERF_EF_UPDATE); > > - > > - for (i = 0; i < box->n_events; i++) { > > - if (event == box->event_list[i]) { > > - --box->n_events; > > - break; > > - } > > - } > > I'll apply this fix - but could we please also make sure box->event_list[] > _always_ get initialized to a sane state? >
box is allocated by kzalloc_node. It should be always initialized to a sane state.
But the previous code only update n_events, and forget to update event_list in event add. That triggers the bug in event del.
Thanks, Kan
> If it had a proper zero initial value in box->n_events the bug would not > have triggered. So struct intel_uncore_box initialization appears to be > sloppy, and that should be looked at as well... > > Thanks, > > Ingo
| |