From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753354AbdKXI3w convert rfc822-to-8bit (ORCPT ); Fri, 24 Nov 2017 03:29:52 -0500 Received: from mga02.intel.com ([134.134.136.20]:45029 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753268AbdKXI3r (ORCPT ); Fri, 24 Nov 2017 03:29:47 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,445,1505804400"; d="scan'208";a="5820660" From: "Reshetova, Elena" To: Zdenek Kabelac , "dm-devel@redhat.com" CC: "linux-bcache@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-raid@vger.kernel.org" , "kent.overstreet@gmail.com" , "koverstreet@google.com" , "ejt@redhat.com" , "snitzer@redhat.com" , "shli@kernel.org" , "agk@redhat.com" , "peterz@infradead.org" , "keescook@chromium.org" Subject: RE: [PATCH 4/4] dm: convert table_device.count from atomic_t to refcount_t Thread-Topic: [PATCH 4/4] dm: convert table_device.count from atomic_t to refcount_t Thread-Index: AQHTSXZ6Wa+XYi/1oUGWMgUxLVsMuaMiSfKAgAEcvSA= Date: Fri, 24 Nov 2017 08:29:42 +0000 Message-ID: <2236FBA76BA1254E88B949DDB74E612B802C1F4D@IRSMSX102.ger.corp.intel.com> References: <1508485059-21881-1-git-send-email-elena.reshetova@intel.com> <1508485059-21881-5-git-send-email-elena.reshetova@intel.com> <92f8c04f-ef1a-c41b-e82f-8d68a3ec84b5@redhat.com> In-Reply-To: <92f8c04f-ef1a-c41b-e82f-8d68a3ec84b5@redhat.com> 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: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMGIyNzhmMWUtMWY5Zi00OTcxLWJiNzktOWMyYjM3YjQwZGQzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJaUnlocWhjUW00ZjZmd2x2ZGZzMWtqRFZVanNFWFl0dnJ6ZmkwNjE1dlorOTFkK3dCamhYd0ZRYlFiOGVhNTl5In0= x-originating-ip: [163.33.239.180] 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 > Dne 20.10.2017 v 09:37 Elena Reshetova napsal(a): > > 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 table_device.count is used as pure reference counter. > > Convert it to refcount_t and fix up the operations. > > > > Suggested-by: Kees Cook > > Reviewed-by: David Windsor > > Reviewed-by: Hans Liljestrand > > Signed-off-by: Elena Reshetova > > --- > > drivers/md/dm.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/md/dm.c b/drivers/md/dm.c > > index 4be8532..be12f3f 100644 > > --- a/drivers/md/dm.c > > +++ b/drivers/md/dm.c > > @@ -24,6 +24,7 @@ > > #include > > #include > > #include > > +#include > > > > #define DM_MSG_PREFIX "core" > > > > @@ -98,7 +99,7 @@ struct dm_md_mempools { > > > > struct table_device { > > struct list_head list; > > - atomic_t count; > > + refcount_t count; > > struct dm_dev dm_dev; > > }; > > > > @@ -685,10 +686,11 @@ int dm_get_table_device(struct mapped_device *md, > dev_t dev, fmode_t mode, > > > > format_dev_t(td->dm_dev.name, dev); > > > > - atomic_set(&td->count, 0); > > + refcount_set(&td->count, 1); > > list_add(&td->list, &md->table_devices); > > + } else { > > + refcount_inc(&td->count); > > } > > - atomic_inc(&td->count); > > mutex_unlock(&md->table_devices_lock); > > > > > NACK > > This patch (2a0b4682e09d76466f7b8f5e347ae2ff02f033af) currently breaks > accounting of opened devices. > > I.e. multisegment device (target with 3 segments is not properly accounted) Could you please explain what exactly happens (i.e. missing/wrong increment?) or provide a error dump? By looking at the code, I don't see where the change in the reference counting could have caused this. Best Regards, Elena. > > > Patch needs reworking and users of 'dm' and 4.15-rc0 kernel should rather > switch back to 4.14 ATM as it's unclear which other parts can be affected. > > Zdenek > > > *result = &td->dm_dev; > > @@ -701,7 +703,7 @@ void dm_put_table_device(struct mapped_device *md, > struct dm_dev *d) > > struct table_device *td = container_of(d, struct table_device, > dm_dev); > > > > mutex_lock(&md->table_devices_lock); > > - if (atomic_dec_and_test(&td->count)) { > > + if (refcount_dec_and_test(&td->count)) { > > close_table_device(td, md); > > list_del(&td->list); > > kfree(td); > > @@ -718,7 +720,7 @@ static void free_table_devices(struct list_head *devices) > > struct table_device *td = list_entry(tmp, struct > table_device, list); > > > > DMWARN("dm_destroy: %s still exists with %d > references", > > - td->dm_dev.name, atomic_read(&td->count)); > > + td->dm_dev.name, refcount_read(&td->count)); > > kfree(td); > > } > > } > >