From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ig0-f199.google.com (mail-ig0-f199.google.com [209.85.213.199]) by kanga.kvack.org (Postfix) with ESMTP id C6CEA6B025E for ; Mon, 2 May 2016 09:46:50 -0400 (EDT) Received: by mail-ig0-f199.google.com with SMTP id sq19so195684059igc.0 for ; Mon, 02 May 2016 06:46:50 -0700 (PDT) Received: from out1134-203.mail.aliyun.com (out1134-203.mail.aliyun.com. [42.120.134.203]) by mx.google.com with ESMTP id np9si4090406igc.19.2016.05.02.06.46.46 for ; Mon, 02 May 2016 06:46:47 -0700 (PDT) Message-ID: <57275B71.8000907@emindsoft.com.cn> Date: Mon, 02 May 2016 21:51:45 +0800 From: Chen Gang MIME-Version: 1.0 Subject: Re: [PATCH] mm/kasan/kasan.h: Fix boolean checking issue for kasan_report_enabled() References: <1462167374-6321-1-git-send-email-chengang@emindsoft.com.cn> <572735EB.8030300@emindsoft.com.cn> <572747C2.5040009@emindsoft.com.cn> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: owner-linux-mm@kvack.org List-ID: To: Alexander Potapenko Cc: Dmitry Vyukov , Andrew Morton , Andrey Ryabinin , kasan-dev , LKML , "linux-mm@kvack.org" , Chen Gang On 5/2/16 20:42, Alexander Potapenko wrote: > On Mon, May 2, 2016 at 2:27 PM, Chen Gang wrote: >> On 5/2/16 19:21, Dmitry Vyukov wrote: >>> >>> Signed counter looks good to me. >> >> Oh, sorry, it seems a little mess (originally, I need let the 2 patches >> in one patch set). >> >> If what Alexander's idea is OK (if I did not misunderstand), I guess, >> unsigned counter is still necessary. > I don't think it's critical for us to use an unsigned counter. > If we increment the counter in kasan_disable_current() and decrement > it in kasan_enable_current(), as Dmitry suggested, we'll be naturally > using only positive integers for the counter. > If the counter drops below zero, or exceeds a certain number (say, > 20), we can immediately issue a warning. > OK, thanks. And for "kasan_depth == 1", I guess, its meaning is related with kasan_depth[++|--] in kasan_[en|dis]able_current(): - If kasan_depth++ in kasan_enable_current() with preventing overflow/ underflow, it means "we always want to disable KASAN, if CONFIG_KASAN is not under arm64 or x86_64". - If kasan_depth-- in kasan_enable_current() with preventing overflow/ underflow, it means "we can enable KASAN if CONFIG_KASAN, but firstly we disable it, if it is not under arm64 or x86_64". For me, I don't know which one is correct (or my whole 'guess' is incorrect). Could any members provide your ideas? >>> We can both issue a WARNING and prevent the actual overflow/underflow. >>> But I don't think that there is any sane way to handle the bug (other >>> than just fixing the unmatched disable/enable). If we ignore an >>> excessive disable, then we can end up with ignores enabled >>> permanently. If we ignore an excessive enable, then we can end up with >>> ignores enabled when they should not be enabled. The main point here >>> is to bark loudly, so that the unmatched annotations are noticed and >>> fixed. >>> >> >> How about BUG_ON()? > As noted by Dmitry in an offline discussion, we shouldn't bail out as > long as it's possible to proceed, otherwise the kernel may become very > hard to debug. > A mismatching annotation isn't a case in which we can't proceed with > the execution. OK, thanks. I guess, we are agree with each other: "We can both issue a WARNING and prevent the actual overflow/underflow.". Thanks. -- Chen Gang (e??a??) Managing Natural Environments is the Duty of Human Beings. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753832AbcEBNrC (ORCPT ); Mon, 2 May 2016 09:47:02 -0400 Received: from out4434.biz.mail.alibaba.com ([47.88.44.34]:20387 "EHLO out4434.biz.mail.alibaba.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751623AbcEBNqz (ORCPT ); Mon, 2 May 2016 09:46:55 -0400 X-Greylist: delayed 9579 seconds by postgrey-1.27 at vger.kernel.org; Mon, 02 May 2016 09:46:55 EDT X-Alimail-AntiSpam: AC=CONTINUE;BC=0.07585356|-1;FP=0|0|0|0|0|-1|-1|-1;HT=e02c03285;MF=chengang@emindsoft.com.cn;NM=1;PH=DS;RN=8;RT=8;SR=0;TI=SMTPD_----4lAcNr9_1462196800; Message-ID: <57275B71.8000907@emindsoft.com.cn> Date: Mon, 02 May 2016 21:51:45 +0800 From: Chen Gang User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Alexander Potapenko CC: Dmitry Vyukov , Andrew Morton , Andrey Ryabinin , kasan-dev , LKML , "linux-mm@kvack.org" , Chen Gang Subject: Re: [PATCH] mm/kasan/kasan.h: Fix boolean checking issue for kasan_report_enabled() References: <1462167374-6321-1-git-send-email-chengang@emindsoft.com.cn> <572735EB.8030300@emindsoft.com.cn> <572747C2.5040009@emindsoft.com.cn> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 5/2/16 20:42, Alexander Potapenko wrote: > On Mon, May 2, 2016 at 2:27 PM, Chen Gang wrote: >> On 5/2/16 19:21, Dmitry Vyukov wrote: >>> >>> Signed counter looks good to me. >> >> Oh, sorry, it seems a little mess (originally, I need let the 2 patches >> in one patch set). >> >> If what Alexander's idea is OK (if I did not misunderstand), I guess, >> unsigned counter is still necessary. > I don't think it's critical for us to use an unsigned counter. > If we increment the counter in kasan_disable_current() and decrement > it in kasan_enable_current(), as Dmitry suggested, we'll be naturally > using only positive integers for the counter. > If the counter drops below zero, or exceeds a certain number (say, > 20), we can immediately issue a warning. > OK, thanks. And for "kasan_depth == 1", I guess, its meaning is related with kasan_depth[++|--] in kasan_[en|dis]able_current(): - If kasan_depth++ in kasan_enable_current() with preventing overflow/ underflow, it means "we always want to disable KASAN, if CONFIG_KASAN is not under arm64 or x86_64". - If kasan_depth-- in kasan_enable_current() with preventing overflow/ underflow, it means "we can enable KASAN if CONFIG_KASAN, but firstly we disable it, if it is not under arm64 or x86_64". For me, I don't know which one is correct (or my whole 'guess' is incorrect). Could any members provide your ideas? >>> We can both issue a WARNING and prevent the actual overflow/underflow. >>> But I don't think that there is any sane way to handle the bug (other >>> than just fixing the unmatched disable/enable). If we ignore an >>> excessive disable, then we can end up with ignores enabled >>> permanently. If we ignore an excessive enable, then we can end up with >>> ignores enabled when they should not be enabled. The main point here >>> is to bark loudly, so that the unmatched annotations are noticed and >>> fixed. >>> >> >> How about BUG_ON()? > As noted by Dmitry in an offline discussion, we shouldn't bail out as > long as it's possible to proceed, otherwise the kernel may become very > hard to debug. > A mismatching annotation isn't a case in which we can't proceed with > the execution. OK, thanks. I guess, we are agree with each other: "We can both issue a WARNING and prevent the actual overflow/underflow.". Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.