From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751956AbcFIOFY (ORCPT ); Thu, 9 Jun 2016 10:05:24 -0400 Received: from mail-db3on0133.outbound.protection.outlook.com ([157.55.234.133]:45920 "EHLO emea01-db3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750810AbcFIOFW (ORCPT ); Thu, 9 Jun 2016 10:05:22 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=aryabinin@virtuozzo.com; Subject: [PATCH] mm: mempool: kasan: don't poot mempool objects in quarantine To: Alexander Potapenko , , , , , , , , , References: <1464785606-20349-1-git-send-email-glider@google.com> <574F0BB6.1040400@virtuozzo.com> CC: , , , Kuthonuzo Luruo From: Andrey Ryabinin Message-ID: <575977C3.1010905@virtuozzo.com> Date: Thu, 9 Jun 2016 17:05:55 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <574F0BB6.1040400@virtuozzo.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [195.214.232.10] X-ClientProxiedBy: AMSPR02CA0012.eurprd02.prod.outlook.com (10.242.225.140) To VI1PR0801MB1311.eurprd08.prod.outlook.com (10.167.197.149) X-MS-Office365-Filtering-Correlation-Id: 9da1a10d-6e35-44fd-c8cd-08d3906f12b5 X-Microsoft-Exchange-Diagnostics: 1;VI1PR0801MB1311;2:wm2O0Hyu6yjxkUY0cioraNxdPU7eIcCg9f+1qlBvMJ+0E1muaSm5lNG8mNhdeL+gkDsWFVA/rL//ArdAievBDBI9TQxU76Y++SBN84Sw1kuZWgFTzKQricUMntVVd3R1SjU+C5/LUe2ylwzHMt9BwFnlyX9RE7lySuZyBboCtY2/9dlugC0JxZcKH6v92RiA;3:H3+PYdyUn7w4ccCx2b6d7qZK6uns8pX73clT86D9kQXsYk5JQG20qOoJ0V2i/sNbdAYTLBbGLrnJGQUGR3d9Im4nITeka1Mr4RdZTcQxby1fU+he+pgrkRbzdeUwjvRo;25:izmRCt8gVZhtIu4GMLGWwBCRUIQYOZuz/nUfNVcP3M2V/TFYxiqnY8e2AZyhgozkaflfuEOHnKIqh1V6FhjR6wD3icISjqGbZ5/gXrVoUt/Kga9KUZPY7O1+agE1jTfVKWYYsjP4sHROsoqmnoYrxRKbxTt6/rVL+6NSXSU8pUfkvN5PZBqI2pXIedcU17NVOLBf80fYHhvU3CnpExa9jPvbYF+yABB+SGSFyRZKlf38igSmUVsqAqT8AMQceyoBnUXdVvBQijbpuOfNdI0rGtwIqIgiB2UuuSi3vSOv7ohnvPG7NdiUcyfPrKrCh4boLwqV0xjpdXcInxnBLb5xJmg8xGAbc/YQRgnUWM2PlbjJbN911OFBoECwJjR5GRtZwW7nnNXhEFaIq35eynLBNQ== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:VI1PR0801MB1311; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(227479698468861)(131327999870524); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040130)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6041072)(6043046);SRVR:VI1PR0801MB1311;BCL:0;PCL:0;RULEID:;SRVR:VI1PR0801MB1311; X-Microsoft-Exchange-Diagnostics: 1;VI1PR0801MB1311;4:16/4r2ERhDao7jNXitmYJ912FCUb1ui35cl9TtcsqAbIvK65IVrYzr/58YNPjrfXTva9+aFy42jAtS5RUmIE863v/atDp8voS6ODgyvdnTnA2TkKPsEF/5ueiAz+9xqT4Ng7UAYY6p6X6t6eD6PrM507Lcw1vFhePcGCwj+LKEBfoaUq6m0iF4vjDJKhqhQ/JDfGt5P4uiocPZxuA1Vu6LPgQGpOD+UwkQ0aZwlg7F1/qD22KZNzTIa69ARzos8XUHLiCcP+M0TKeCIWLbdDGMNReAy972LVqpWE1ytZItv9a72xd7wEHEdv3irSuvqkLarcC7QsGh+71PRlVsZyX37pbdXs7KuuINStWj5/R/18P24iGTtlzHqZ3JiO6NOPKxTY2zM4Uk1TsesnfEmIhszc/P5/T3sk3D4VH69x/DW71xSgYPJ/NFxBhZzxUP8IxzK79/NFxJmDmaklANgGApoQpn5bGw1SQl7z1aqmCj7SFk4HK8luOOzo03DOFv5i X-Forefront-PRVS: 0968D37274 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(189002)(199003)(377454003)(24454002)(6116002)(230700001)(105586002)(3846002)(65806001)(229853001)(23746002)(81166006)(5008740100001)(106356001)(65816999)(87266999)(5004730100002)(586003)(76176999)(50466002)(65956001)(8676002)(66066001)(92566002)(50986999)(68736007)(54356999)(189998001)(42186005)(5001770100001)(2950100001)(47776003)(86362001)(19580395003)(83506001)(80316001)(99136001)(36756003)(59896002)(2201001)(19580405001)(97736004)(64126003)(81156014)(77096005)(4001350100001)(33656002)(4326007)(2906002)(101416001)(921003)(2101003)(1121003)(83996005);DIR:OUT;SFP:1102;SCL:1;SRVR:VI1PR0801MB1311;H:[10.30.19.223];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;VI1PR0801MB1311;23:6amfLB8H/mAnPJXf99uFVmMW5cS9ALH98Ng?= =?Windows-1252?Q?jN/JxxIKAd7P5LeYeTkr9d2qJCqJXt+thzsw42pFckS9dXSI0FecoPDS?= =?Windows-1252?Q?d9SSp0ht1Mvavjz5jziAqKDpfOsljbkrrjX0xaYbvQzWuHIBnvtXGw06?= =?Windows-1252?Q?MyE121nxCrdl5ywzP8U0IkUElD7SmN+Z8Ke+CXcp1C1tKyjTQZi7IROj?= =?Windows-1252?Q?KI6NRiPtZLG3VkDWjnPhGyER96Ft1I/iDvdF2nUuZvE5t5kudStW/AcH?= =?Windows-1252?Q?b21ONAeaOa3NAFIJT1EDvh4XW676PwZMEjaui+mNM1Q1efsW3yJPm0Oa?= =?Windows-1252?Q?JfH+hCHLmRAp1/NadZMeFhvgxwNqMDU1wQgiqqJaASUX4txc3ATolSQk?= =?Windows-1252?Q?PP17yPXOrH6mm1CCBvWGTHh9whZo4bwoBIGeFUhiQJYL/91WEBnv3lZc?= =?Windows-1252?Q?9AvMZk2eXd5yVYNAXUlqJj4HYqdkJT2COu87xBcFF2IqZ8FRWBY5pP25?= =?Windows-1252?Q?sOkALm29DYoLylZ9jM999V61MxB6x2Hs+I0Sq5wH3DytSjUP7J1mxYCo?= =?Windows-1252?Q?KMEN3qrlnhMGvZ6Uf2w7S41krbFgggIK2RNl3+cpbP5DT5WfLHDjs/cF?= =?Windows-1252?Q?J8kAzwSwyJ4TLHYBuqI8zVr3fYZTrT6A5E7LXFgYbmXtxfLPXspAmKei?= =?Windows-1252?Q?/FUS0UDHKWYXgxnDoiU4bda/cyTh0N/Cd9i3aiAgZ7/Lj6ufcNx1N4EF?= =?Windows-1252?Q?qzZ7TbVEyGbhn8XeotLbFqrDNfeeaZyEOPa2EBcYo4W6CWnOHoYkJgfj?= =?Windows-1252?Q?eyRcJVc6LWaxED4wBUXCxQhu/r7frG4zZzy630z1Y2f1M13sQlFZhYUV?= =?Windows-1252?Q?wtZSP1qL1fqj+B+Cptex43x3IL9GlbXZN3cAU2RIX7bFyYQsFQJdJe4y?= =?Windows-1252?Q?RqC6s4bVt8Om/VGTJxiPSN2wBxaJqykfNw7Dsy2Kgt/rLYlmWZNSt8xM?= =?Windows-1252?Q?+rDRCJvtUgwBXlAx5sCT61u4XRQCTe5Fl0grT5oc2xsL6QAYsWuz3qyv?= =?Windows-1252?Q?5uvYDoiSh5Ua6TuGLjdeJMnLF9KDIApdaDienLc2UQsWAdO4W1wDmNBZ?= =?Windows-1252?Q?7a5RSqLfBloYaCFyxpXLY9ychOpcs08ISRfMSm+qPU4v0JSydCJHalmA?= =?Windows-1252?Q?yYkYDXkWuyv4aDHKRRDeTKYBhtvJZFsjV5yQZXB2ZGspFXSJmEB1u85Y?= =?Windows-1252?Q?+pI7UrRKETzJWweuXTIqlu/KTOCbs181x4nz1Ioh9r6v6DnJCiTldQRR?= =?Windows-1252?Q?tDtNbGcyQaMgzLzGhF1RnrkKKHWykRbMJsG7KbkiVOlYqYdrU/zgyia7?= =?Windows-1252?Q?ZfzIqz+6yyTBKy5wmNv/BGJOVUWAZVOQAYR1BdRCcfQEFhu4MeN8vbI3?= =?Windows-1252?Q?1REip0r4VfsJiEIlFbdk2t6/lyWi3TzmXV8fy5D1/IQMGoA+l57+XCHF?= =?Windows-1252?Q?RVR32zLFRYp2fc/EfObklmmEFy/JvMzYmco1y1ybcx3IfOX02YA=3D?= =?Windows-1252?Q?=3D?= X-Microsoft-Exchange-Diagnostics: 1;VI1PR0801MB1311;5:nBasT9bNr7rDbeRrNG4jy5xX/PHVdYfBqnfv+xUvTXVsgwdLj/M9F1xgi0M7GjszMTnHNCnoNpZ9N1wZz1IalRByTguGVIjY/UQHAv9vRu4d61VwdeAJfxJawWXwd0NQVLtclI6Joq40veY1utx6/g==;24:jOZIJFY6whbhOxuNr9ASYgaQJFsTpix8mJ9h9+O1gpsX7BTmyMW47Rdx70YTNQBCk2Ikqs1jokQp/AhHMwJsza+4Lvn1pHDW1nGcUkVaxeM=;7:uY7xfJvgGjMV1HVjzgfPlsq2NREecb+ZGo1ricExRqCL/xP3JI5R4JrFzHnjVXpRfqnsfb41LXPbLZ3T9qkS8T4ECjRhkXT/hGMYDNjrmO8pl1EYCaUO9RYVBS1N3IdAlYKD9PgNsUgbWVpwcXjX5Y19Fj/CDh/HkHYx0WiqBJB4d8js5LpQd6XzMTNrJjM/PeKUFQ5GMrADev2r0kMnYw==;20:f5syRMkfcMZETNiPd7Y9GHCtO2zxV1TVrVoe84K8VS6sThMmsXrFFI35nQAY3kdGc8pJSzxP9jo19TCVws6ZfDY0w8Zz3ia6w5d2krLCROyqlPLIKdUjlaSvVewelqBQMpC/KcyQeaXdQathRvOW5NR30XxO619QcvKw9C8nySA= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Jun 2016 14:05:05.8573 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0801MB1311 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/01/2016 07:22 PM, Andrey Ryabinin wrote: > > > On 06/01/2016 03:53 PM, Alexander Potapenko wrote: >> To avoid draining the mempools, KASAN shouldn't put the mempool elements >> into the quarantine upon mempool_free(). > > Correct, but unfortunately this patch doesn't fix that. > So, I made this: From: Andrey Ryabinin Subject: [PATCH] mm: mempool: kasan: don't poot mempool objects in quarantine Currently we may put reserved by mempool elements into quarantine via kasan_kfree(). This is totally wrong since quarantine may really free these objects. So when mempool will try to use such element, use-after-free will happen. Or mempool may decide that it no longer need that element and double-free it. So don't put object into quarantine in kasan_kfree(), just poison it. Rename kasan_kfree() to kasan_poison_kfree() to respect that. Also, we shouldn't use kasan_slab_alloc()/kasan_krealloc() in kasan_unpoison_element() because those functions may update allocation stacktrace. This would be wrong for the most of the remove_element call sites. (The only call site where we may want to update alloc stacktrace is in mempool_alloc(). Kmemleak solves this by calling kmemleak_update_trace(), so we could make something like that too. But this is out of scope of this patch). Fixes: 55834c59098d ("mm: kasan: initial memory quarantine implementation") Signed-off-by: Andrey Ryabinin Reported-by: Kuthonuzo Luruo --- include/linux/kasan.h | 11 +++++++---- mm/kasan/kasan.c | 6 +++--- mm/mempool.c | 12 ++++-------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/include/linux/kasan.h b/include/linux/kasan.h index 611927f..ac4b3c4 100644 --- a/include/linux/kasan.h +++ b/include/linux/kasan.h @@ -59,14 +59,13 @@ void kasan_poison_object_data(struct kmem_cache *cache, void *object); void kasan_kmalloc_large(const void *ptr, size_t size, gfp_t flags); void kasan_kfree_large(const void *ptr); -void kasan_kfree(void *ptr); +void kasan_poison_kfree(void *ptr); void kasan_kmalloc(struct kmem_cache *s, const void *object, size_t size, gfp_t flags); void kasan_krealloc(const void *object, size_t new_size, gfp_t flags); void kasan_slab_alloc(struct kmem_cache *s, void *object, gfp_t flags); bool kasan_slab_free(struct kmem_cache *s, void *object); -void kasan_poison_slab_free(struct kmem_cache *s, void *object); struct kasan_cache { int alloc_meta_offset; @@ -76,6 +75,9 @@ struct kasan_cache { int kasan_module_alloc(void *addr, size_t size); void kasan_free_shadow(const struct vm_struct *vm); +size_t ksize(const void *); +static inline void kasan_unpoison_slab(const void *ptr) { ksize(ptr); } + #else /* CONFIG_KASAN */ static inline void kasan_unpoison_shadow(const void *address, size_t size) {} @@ -102,7 +104,7 @@ static inline void kasan_poison_object_data(struct kmem_cache *cache, static inline void kasan_kmalloc_large(void *ptr, size_t size, gfp_t flags) {} static inline void kasan_kfree_large(const void *ptr) {} -static inline void kasan_kfree(void *ptr) {} +static inline void kasan_poison_kfree(void *ptr) {} static inline void kasan_kmalloc(struct kmem_cache *s, const void *object, size_t size, gfp_t flags) {} static inline void kasan_krealloc(const void *object, size_t new_size, @@ -114,11 +116,12 @@ static inline bool kasan_slab_free(struct kmem_cache *s, void *object) { return false; } -static inline void kasan_poison_slab_free(struct kmem_cache *s, void *object) {} static inline int kasan_module_alloc(void *addr, size_t size) { return 0; } static inline void kasan_free_shadow(const struct vm_struct *vm) {} +static inline void kasan_unpoison_slab(const void *ptr) { } + #endif /* CONFIG_KASAN */ #endif /* LINUX_KASAN_H */ diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c index 28439ac..6845f92 100644 --- a/mm/kasan/kasan.c +++ b/mm/kasan/kasan.c @@ -508,7 +508,7 @@ void kasan_slab_alloc(struct kmem_cache *cache, void *object, gfp_t flags) kasan_kmalloc(cache, object, cache->object_size, flags); } -void kasan_poison_slab_free(struct kmem_cache *cache, void *object) +static void kasan_poison_slab_free(struct kmem_cache *cache, void *object) { unsigned long size = cache->object_size; unsigned long rounded_up_size = round_up(size, KASAN_SHADOW_SCALE_SIZE); @@ -626,7 +626,7 @@ void kasan_krealloc(const void *object, size_t size, gfp_t flags) kasan_kmalloc(page->slab_cache, object, size, flags); } -void kasan_kfree(void *ptr) +void kasan_poison_kfree(void *ptr) { struct page *page; @@ -636,7 +636,7 @@ void kasan_kfree(void *ptr) kasan_poison_shadow(ptr, PAGE_SIZE << compound_order(page), KASAN_FREE_PAGE); else - kasan_slab_free(page->slab_cache, ptr); + kasan_poison_slab_free(page->slab_cache, ptr); } void kasan_kfree_large(const void *ptr) diff --git a/mm/mempool.c b/mm/mempool.c index 9e075f8..8f65464 100644 --- a/mm/mempool.c +++ b/mm/mempool.c @@ -104,20 +104,16 @@ static inline void poison_element(mempool_t *pool, void *element) static void kasan_poison_element(mempool_t *pool, void *element) { - if (pool->alloc == mempool_alloc_slab) - kasan_poison_slab_free(pool->pool_data, element); - if (pool->alloc == mempool_kmalloc) - kasan_kfree(element); + if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc) + kasan_poison_kfree(element); if (pool->alloc == mempool_alloc_pages) kasan_free_pages(element, (unsigned long)pool->pool_data); } static void kasan_unpoison_element(mempool_t *pool, void *element, gfp_t flags) { - if (pool->alloc == mempool_alloc_slab) - kasan_slab_alloc(pool->pool_data, element, flags); - if (pool->alloc == mempool_kmalloc) - kasan_krealloc(element, (size_t)pool->pool_data, flags); + if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc) + kasan_unpoison_slab(element); if (pool->alloc == mempool_alloc_pages) kasan_alloc_pages(element, (unsigned long)pool->pool_data); } -- 2.7.3