From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751327AbdJWKWJ convert rfc822-to-8bit (ORCPT ); Mon, 23 Oct 2017 06:22:09 -0400 Received: from mga02.intel.com ([134.134.136.20]:65519 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751150AbdJWKWH (ORCPT ); Mon, 23 Oct 2017 06:22:07 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,422,1503385200"; d="scan'208";a="141196995" From: "Reshetova, Elena" To: Thomas Gleixner CC: "mingo@redhat.com" , "linux-kernel@vger.kernel.org" , "linux-fsdevel@vger.kernel.org" , "peterz@infradead.org" , "gregkh@linuxfoundation.org" , "viro@zeniv.linux.org.uk" , "tj@kernel.org" , "hannes@cmpxchg.org" , "lizefan@huawei.com" , "acme@kernel.org" , "alexander.shishkin@linux.intel.com" , "eparis@redhat.com" , "akpm@linux-foundation.org" , "arnd@arndb.de" , "luto@kernel.org" , "keescook@chromium.org" , "dvhart@infradead.org" , "ebiederm@xmission.com" , "linux-mm@kvack.org" , "axboe@kernel.dk" Subject: RE: [PATCH 01/15] sched: convert sighand_struct.count to refcount_t Thread-Topic: [PATCH 01/15] sched: convert sighand_struct.count to refcount_t Thread-Index: AQHTSZ0yz/zi7QlHYEejINzTncYKnaLsnjEAgASeHyA= Date: Mon, 23 Oct 2017 10:22:01 +0000 Message-ID: <2236FBA76BA1254E88B949DDB74E612B802B4359@IRSMSX102.ger.corp.intel.com> References: <1508501757-15784-1-git-send-email-elena.reshetova@intel.com> <1508501757-15784-2-git-send-email-elena.reshetova@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOTUyNTE1NzktY2NjNC00OTUzLWE0NTItZDJjN2ZjMzYzMjcxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJKVkdOQ1U0bUdtMkkycFhpZkh1RWs5VzZ0K0dtRVhyaEJcL0R5UVRRTjVxMjZTbFlmOGpHRU0xV2twXC9iRHpadmgifQ== x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On Fri, 20 Oct 2017, Elena Reshetova wrote: > > > atomic_t variables are currently used to implement reference > > counters with the following properties: > > - counter is initialized to 1 using atomic_set() > > - a resource is freed upon counter reaching zero > > - once counter reaches zero, its further > > increments aren't allowed > > - counter schema uses basic atomic operations > > (set, inc, inc_not_zero, dec_and_test, etc.) > > > > Such atomic variables should be converted to a newly provided > > refcount_t type and API that prevents accidental counter overflows > > and underflows. This is important since overflows and underflows > > can lead to use-after-free situation and be exploitable. > > > > The variable sighand_struct.count is used as pure reference counter. > > This still does not mention that atomic_t != recfcount_t ordering wise and > why you think that this does not matter in that use case. > > > And looking deeper: > > > @@ -1381,7 +1381,7 @@ static int copy_sighand(unsigned long clone_flags, > struct task_struct *tsk) > > struct sighand_struct *sig; > > > > if (clone_flags & CLONE_SIGHAND) { > > - atomic_inc(¤t->sighand->count); > > + refcount_inc(¤t->sighand->count); > > return 0; > > > void __cleanup_sighand(struct sighand_struct *sighand) > > { > > - if (atomic_dec_and_test(&sighand->count)) { > > + if (refcount_dec_and_test(&sighand->count)) { > > How did you make sure that these atomic operations have no other > serialization effect and can be replaced with refcount? What serialization effects? Are you taking about smth else than memory ordering? For memory ordering my current hope is that we can just make refcount_t to use same strict atomic primitives and then it would not make any difference. I think this would be the simplest way for everyone since I think even some maintainers are having issues understanding all the implications of "relaxed" ordering. Best Regards, Elena > > I complained about that before and Peter explained it to you in great > length, but you just resend the same thing again. Where is the correctness > analysis? Seriously, for this kind of stuff it's not sufficient to use a > coccinelle script and copy boiler plate change logs and be done with it. > > Thanks, > > tglx From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: "Reshetova, Elena" To: Thomas Gleixner CC: "mingo@redhat.com" , "linux-kernel@vger.kernel.org" , "linux-fsdevel@vger.kernel.org" , "peterz@infradead.org" , "gregkh@linuxfoundation.org" , "viro@zeniv.linux.org.uk" , "tj@kernel.org" , "hannes@cmpxchg.org" , "lizefan@huawei.com" , "acme@kernel.org" , "alexander.shishkin@linux.intel.com" , "eparis@redhat.com" , "akpm@linux-foundation.org" , "arnd@arndb.de" , "luto@kernel.org" , "keescook@chromium.org" , "dvhart@infradead.org" , "ebiederm@xmission.com" , "linux-mm@kvack.org" , "axboe@kernel.dk" Subject: RE: [PATCH 01/15] sched: convert sighand_struct.count to refcount_t Date: Mon, 23 Oct 2017 10:22:01 +0000 Message-ID: <2236FBA76BA1254E88B949DDB74E612B802B4359@IRSMSX102.ger.corp.intel.com> References: <1508501757-15784-1-git-send-email-elena.reshetova@intel.com> <1508501757-15784-2-git-send-email-elena.reshetova@intel.com> In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Sender: owner-linux-mm@kvack.org List-ID: > On Fri, 20 Oct 2017, Elena Reshetova wrote: >=20 > > atomic_t variables are currently used to implement reference > > counters with the following properties: > > - counter is initialized to 1 using atomic_set() > > - a resource is freed upon counter reaching zero > > - once counter reaches zero, its further > > increments aren't allowed > > - counter schema uses basic atomic operations > > (set, inc, inc_not_zero, dec_and_test, etc.) > > > > Such atomic variables should be converted to a newly provided > > refcount_t type and API that prevents accidental counter overflows > > and underflows. This is important since overflows and underflows > > can lead to use-after-free situation and be exploitable. > > > > The variable sighand_struct.count is used as pure reference counter. >=20 > This still does not mention that atomic_t !=3D recfcount_t ordering wise = and > why you think that this does not matter in that use case. > > > And looking deeper: >=20 > > @@ -1381,7 +1381,7 @@ static int copy_sighand(unsigned long clone_flags= , > struct task_struct *tsk) > > struct sighand_struct *sig; > > > > if (clone_flags & CLONE_SIGHAND) { > > - atomic_inc(¤t->sighand->count); > > + refcount_inc(¤t->sighand->count); > > return 0; >=20 > > void __cleanup_sighand(struct sighand_struct *sighand) > > { > > - if (atomic_dec_and_test(&sighand->count)) { > > + if (refcount_dec_and_test(&sighand->count)) { >=20 > How did you make sure that these atomic operations have no other > serialization effect and can be replaced with refcount? What serialization effects? Are you taking about smth else than memory ordering?=20 For memory ordering my current hope is that we can just make refcount_t to use same strict atomic primitives and then it would not make any differe= nce. I think this would be the simplest way for everyone since I think even some= maintainers are having issues understanding all the implications of "relaxed" ordering.= =20 Best Regards, Elena >=20 > I complained about that before and Peter explained it to you in great > length, but you just resend the same thing again. Where is the correctnes= s > analysis? Seriously, for this kind of stuff it's not sufficient to use a > coccinelle script and copy boiler plate change logs and be done with it. >=20 > Thanks, >=20 > tglx -- 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