On Mon, May 2, 2016 at 1:20 PM, Chen Gang <[email protected]> wrote: > On 5/2/16 18:49, Alexander Potapenko wrote: >> On Mon, May 2, 2016 at 7:35 AM, <[email protected]> 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 <[email protected]> >> >> Acked-by: Alexander Potapenko <[email protected]> Nacked-by: Alexander Potapenko <[email protected]> >> > > 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. > Another patch thread is also related with this patch thread, please help > check. > > And sorry, originally, I did not let the 2 patches in one patches set. > > Thanks. > -- > Chen Gang (陈刚) > > Managing Natural Environments is the Duty of Human Beings. -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
