On 5/2/16 19:23, Alexander Potapenko wrote: > On Mon, May 2, 2016 at 1:20 PM, Chen Gang <cheng...@emindsoft.com.cn> wrote: >> On 5/2/16 18:49, Alexander Potapenko wrote: >>> On Mon, May 2, 2016 at 7:35 AM, <cheng...@emindsoft.com.cn> wrote: >>>> >>>> According to their comments and the kasan_depth's initialization, if >>>> kasan_depth is zero, it means disable. So kasan_depth need consider >>>> about the 0 overflow. >>>> >>>> Also remove useless comments for dummy kasan_slab_free(). >>>> >>>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com> >>> >>> Acked-by: Alexander Potapenko <gli...@google.com> > Nacked-by: Alexander Potapenko <gli...@google.com> >>> >> >> OK, thanks. > Well, on a second thought I take that back, there still might be problems. > I haven't noticed the other CL, and was too hasty reviewing this one. > > As kasan_disable_current() and kasan_enable_current() always go > together, we need to prevent nested calls to them from breaking > everything. > If we ignore some calls to kasan_disable_current() to prevent > overflows, the pairing calls to kasan_enable_current() will bring > |current->kasan_depth| to an invalid state. > > E.g. if I'm understanding your idea correctly, after the following > sequence of calls: > kasan_disable_current(); // #1 > kasan_disable_current(); // #2 > kasan_enable_current(); // #3 > kasan_enable_current(); // #4 > > the value of |current->kasan_depth| will be 2, so a single subsequent > call to kasan_disable_current() won't disable KASAN. > > I think we'd better add BUG checks to bail out if the value of > |current->kasan_depth| is too big or too small. >
For me, BUG_ON is OK. e.g. - BUG_ON(!kasan_depth) as soon as be in kasan_enable_current(). - BUG_ON(!(kasan_depth - 1)) as soon as be in kasan_disable_current(). Welcome another members ideas, if no any additional reply within 3 days, I shall send patch v2 for it. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
