Messages in this thread | | | Date | Mon, 2 May 2016 13:23:03 +0200 | Subject | Re: [PATCH] include/linux/kasan.h: Notice about 0 for kasan_[dis/en]able_current() | From | Alexander Potapenko <> |
| |
On Mon, May 2, 2016 at 1:20 PM, Chen Gang <chengang@emindsoft.com.cn> wrote: > On 5/2/16 18:49, Alexander Potapenko wrote: >> On Mon, May 2, 2016 at 7:35 AM, <chengang@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.5i5j@gmail.com> >> >> Acked-by: Alexander Potapenko <glider@google.com> Nacked-by: Alexander Potapenko <glider@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.
> 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
| |