linux-kernel - Re: [PATCH] mm/kasan/kasan.h: Fix boolean checking issue for kasan_report

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 02 May 2016 23:13:25 +0800
From:	Chen Gang <chengang@...ndsoft.com.cn>
To:	Alexander Potapenko <glider@...gle.com>
CC:	Dmitry Vyukov <dvyukov@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andrey Ryabinin <aryabinin@...tuozzo.com>,
	kasan-dev <kasan-dev@...glegroups.com>,
	LKML <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	Chen Gang <gang.chen.5i5j@...il.com>
Subject: Re: [PATCH] mm/kasan/kasan.h: Fix boolean checking issue for kasan_report_enabled()

On 5/2/16 22:23, Alexander Potapenko wrote:
> On Mon, May 2, 2016 at 3:51 PM, Chen Gang <chengang@...ndsoft.com.cn> wrote:
>>
>> OK, thanks.
>>
>> And for "kasan_depth == 1", I guess, its meaning is related with
>> kasan_depth[++|--] in kasan_[en|dis]able_current():
> Assuming you are talking about the assignment of 1 to kasan_depth in
> /include/linux/init_task.h,
> it's somewhat counterintuitive. I think we just need to replace it
> with kasan_disable_current(), and add a corresponding
> kasan_enable_current() to the end of kasan_init.
>

OK. But it does not look quite easy to use kasan_disable_current() for
INIT_KASAN which is used in INIT_TASK.

If we have to set "kasan_depth == 1", we have to use kasan_depth-- in
kasan_enable_current().
 
>>
>> OK, thanks.
>>
>> I guess, we are agree with each other: "We can both issue a WARNING and
>> prevent the actual overflow/underflow.".
> No, I am not sure think that we need to prevent the overflow.
> As I showed before, this may result in kasan_depth being off even in
> the case kasan_enable_current()/kasan_disable_current() are used
> consistently.

If we don't prevent the overflow, it will have negative effect with the
caller. When we issue an warning, it means the caller's hope fail, but
can not destroy the caller's original work. In our case:

 - Assume "kasan_depth-- for kasan_enable_current()", the first enable
   will let kasan_depth be 0.

 - If we don't prevent the overflow, 2nd enable will cause disable
   effect, which will destroy the caller's original work.

 - Enable/disable mismatch is caused by caller, we can issue warnings,
   and skip it (since it is not caused by us). But we can not generate
   new issues to the system only because of the caller's issue.


Thanks.
-- 
Chen Gang (陈刚)

Managing Natural Environments is the Duty of Human Beings.