From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752421AbcEIK0R (ORCPT ); Mon, 9 May 2016 06:26:17 -0400 Received: from mail-am1on0137.outbound.protection.outlook.com ([157.56.112.137]:25755 "EHLO emea01-am1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751391AbcEIK0P (ORCPT ); Mon, 9 May 2016 06:26:15 -0400 Authentication-Results: kvack.org; dkim=none (message not signed) header.d=none;kvack.org; dmarc=none action=none header.from=virtuozzo.com; Subject: Re: [PATCH v2 1/2] mm, kasan: improve double-free detection To: Kuthonuzo Luruo , , , , , , , References: <20160506114727.GA2571@cherokee.in.rdlabs.hpecorp.net> CC: , , From: Andrey Ryabinin Message-ID: <573065BD.2020708@virtuozzo.com> Date: Mon, 9 May 2016 13:26:05 +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: <20160506114727.GA2571@cherokee.in.rdlabs.hpecorp.net> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [84.47.128.244] X-ClientProxiedBy: AM2PR09CA0009.eurprd09.prod.outlook.com (10.161.22.147) To HE1PR0801MB1306.eurprd08.prod.outlook.com (10.167.247.148) X-MS-Office365-Filtering-Correlation-Id: facf0e31-708a-4eec-83e7-08d377f45795 X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1306;2:P8DKLJdFGE9Amg/YsiH7xN0zCmu9M4Q1SzHF9PalfZRi1Es369dcBJ1674FdI5dkozj+WL7RoWHn6wLUP4T5Q34xGFHxSCdaPTpOw8wMFQpKQPgkNpEnRUvPtKG7Y8JfStAnJOt5Rvsn5WJSANNiMFIFyrW3ftoHjlBOHqr6fv9uY0Xu5hoJvrQ5kt9NzHoh;3:jO5+EkX50qoqrBnmjMyhhIZnE9pcb4AOjQpcH0+JAAucp6nmdELbX6SKv0Jy8G7qlDIuVGlSUhY6XKrI9AAhEM6KGYSiu2r9lvNwA4QKXuOdydN4Ur2YFLjmdzN9kWbW;25:D3y40JMiRNYJLkR5inqinfQxBsJ7MZ4OPllAMzIgA4q1MPsoNziHHLrrI3w1sBtp/rPukgB78XDz+CkUJZ6oB6lKMobYks23rAZRHkHvM8CEwzZnMJEiYgH9AatBFsLrOS5n8SP178lE50okl4thbld8A+NRP3h6LikHL7ti5kMV8UFTdTkdGmXETS2bOvfPBUdY/TkgHH81Nekb06GXHQYSWJsTPYjeqrJIAwNwYXnicb3bpOYUaHfcjmLEHXyJUCJIOeYGFVesl5dBv8YiQmyaqBhgCTPMxs1uCFhpD83fAluDLhOq9KfAG3TtLmqP2STvGCO2RcMrp5a6TiygGaZl4cUUkqTXCYt6vfckarXdPrvUq0KTUveWxwcdV9ihyoDRgBq68bQ8+le3djyejTKup9idcvdLhCGi0FZ2UMRwvtmT2adITmIcOcY2oVGW X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0801MB1306; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040130)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6041072)(6043046);SRVR:HE1PR0801MB1306;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0801MB1306; X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1306;4:bvGmMZYfDRb7E/wJUU/PJ1an76BkW4zKl+b0uWwNInN4OjJZ2K5DDEWYQtccMCoLPW3Jw2+rP//f0vO8Rg+hJJI7HXB/j9ZYsdQKNwmearpNPANRM2ZAYSSMoD7it85Vd9owPYUiJrvMO8RMycADzkIEuJ3SJHKdiuBe/5iCSn0D1IoCqIc5ETWgKICiL57fYGrbntLN9L2FpLQeE01VXl77iVyLZwTnl9AwGpxBOONA0JSCQPU7ZLC09MtSkoHTN6NRF/8v6H4NaHpXF3KmeDaX2uX0fRpRB/jmyp7hPQzUwk5HYdKqibVarPm0rkrp/A9fVOisox+pwZOzoQ+HPHHj4b+W0m35VxwKigu0zzGIZgNk4ZtXM6bdmrEZts/Ftv5PxFP5Ic2IdI0AtTHTkj0NxmoFcM0mE51odP5Z2oo= X-Forefront-PRVS: 0937FB07C5 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(24454002)(377454003)(9170700001)(50986999)(2906002)(230700001)(65816999)(65956001)(3846002)(6116002)(65806001)(5008740100001)(76176999)(87266999)(4326007)(54356999)(36756003)(47776003)(81166005)(5004730100002)(66066001)(77096005)(83506001)(92566002)(23676002)(64126003)(33656002)(80316001)(19580395003)(19580405001)(99136001)(189998001)(50466002)(86362001)(575784001)(2201001)(2950100001)(42186005)(586003)(5001770100001);DIR:OUT;SFP:1102;SCL:1;SRVR:HE1PR0801MB1306;H:[10.52.82.2];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtIRTFQUjA4MDFNQjEzMDY7MjM6L2dGcVVOZUZESG5sMHlERWtoOVpqaGx4?= =?utf-8?B?bnhodDJEenFrT2RST0tVNFFFWTYxRFdLM1o0Z2dTVjE2eDAzcml3eVBsclk1?= =?utf-8?B?YUdveERKVEhaOWZySm0wWGZjVyt4eDZkZUR6bm1VNDU2VDdxV2wvV1JqNnRH?= =?utf-8?B?VjAyQ0V5dmdiWjlVK0RmV2hjaHZndXptZ0h5SjNTRW5SaTl5bzVIektPWUdo?= =?utf-8?B?WTUvOGVoQW91clp5cG1tandwdEE1ZEt0RnUvUTZRVHJ5Wkhpa3pneStZdE9R?= =?utf-8?B?UVBjY0JkYUFHMVVhdHZEYUVYcG16WXNzeG1CZkZPbW1LSi9MVkgyeG1ZK1Za?= =?utf-8?B?aktTdzIwaHArdXVlUElEaG5iVFJiVzF6NnRiMitBOFZ5NllEYlB2eHNYWU8w?= =?utf-8?B?V2JvVVNXSkh5aUdhRjV1WkVZVU5VR2hpdDhOeERGais4YUx5ckVDTmxyb0xy?= =?utf-8?B?dzF4cE9iV09EQmNzYTMzcnpUeVQ1Mnp2Y0ZQMWpRaUxOM2gwV0lJZmxWSXAy?= =?utf-8?B?cDlPbWorYzJNbUUvNGo0VDFEMUlyWnlPSW5weXA1N1E5V1FvUDNwZXZPMHUw?= =?utf-8?B?eEp3UTAvbUgyM3hIWEZ5TlFiRWV4L1ZaSHU3N1pPSGZyUEhDMHEva0NUMldz?= =?utf-8?B?MnowMytidEsvQUFpa0xaR3B4TDJHM2p2QWlwTXdyTTc3VU81UW1wM00vVVgv?= =?utf-8?B?OW9LMDJZdHF4UjhocGlDSUhVTDZKblpnUVg1blBPL2M0VzRlWUdncGllRndO?= =?utf-8?B?KzJKMEY3eWVZcldWWlVncmpQUFBNclNqMVBPdTFjY0JtRmpFZ0tuQzNRVXVZ?= =?utf-8?B?Rzd6MEUyWlcvbGVNcUNodU5XenZOMG53K3BlN09raCs3aG9rWW1VVVVPbnV2?= =?utf-8?B?ZDdzNEo0cFZ2bUhWQkZ5Y212RGxaek9rZ0VqN2daWFNYVjVGWVV3MjZkcEdq?= =?utf-8?B?eXVZTVFPY092QnFvTDhKNzh4VWlUSllDRFBFV3VaTlBqNnN5dDVCRDFGc0lV?= =?utf-8?B?REpUZzFrODZhbkJZL1plbWx5WUhYSGc2NlRqQ0JRYTJ4OCtFN21yQlhmZGEz?= =?utf-8?B?QjRtNzJ4TXhrWE56TVpMRURPa1B0TVNzUmhkYUgxZWw5eTc2RXJSOTJXZUMr?= =?utf-8?B?NEg1SU5BVVRPZm5PdFBjelVMdXNIbGMvbFdvQXZqMlN4QUVZais3MnlzTkNk?= =?utf-8?B?ZDVZSnNNM0NXcnNGRWhLQ2sxdm9GQ1BTdW44WUt6UllvTHhIeDN6U0VYNndn?= =?utf-8?B?ZHBxbGI2ZldubDk3QmdUN2w0K0FpWE9FRFFZUkRoaVY0cTZ1MmJWdzNlY203?= =?utf-8?B?Y0x2SUphbnJyMDJkK0dWRXhoaUFPTTlsVEhBbEhuNHdoTjJhWUNEOUpZZ29k?= =?utf-8?B?WFVwLzU5eU5XOHd4bmpYQll2UDhqRkh6UGNOcGExWEVNNnNHQ25aWEsxblVO?= =?utf-8?B?TXRRODVMa3Vlc25kbjhTZFZvek1WY2ZLQ0FtQW95emFxSm91aVhtRkR1M0pO?= =?utf-8?B?RS9idmNBVVJwR0tuS1BUTDVmWk9lLzFOUFBGQnpsOUNZelQ0TGRBMFdWSGRm?= =?utf-8?Q?7yg?= X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1306;5:bP5sKGSVqK2HIdqOXLTJh3ctviggh6y4rgiuV2Ci3VRblrGkNEG1c0i6abcXLX5daWu8aP/7/sV4INoXDwV3Cde1bo4d8OxPObk/BjT/aUOaQ96+Z2yM+r6gCXwjD+wQcFqwNoims6VignI9kXfSGA==;24:z3TfCfVOfX9AOedOevLo/t+jY4Q483MQ75zC6laR5SFzegUbgEG9ltRakySe9W9rqNucW9htxiJC/3Wn3c2ZlvN2LdV64xprowXj+wVyK3U=;7:6TF3BblsIAgHegiXSP2FL1QBb9iUSlgpcbu0LwZ2QkFjWfoXSAlZ//wIUo7OmR5Y7FHFJzaPD99qXOhUGWKy6omqMSYV3e0kLq4D0XIduW0UWuilKqxfREmSLY0WjdEuOd549kqgss4+9sFLpZilwccZ9T0J5zZkPpkUsl9ahcVDJD89BJKoUT1/dSkVme/I;20:neS+q9Bxj7lnnWM0pgoftXBDow+t6CHmJaGlbv8NwDWD9ytR6whC7746S5wSyl17gUsqMBu5Z8+jvucQWiYrs78psjoSIhJpsImfYPl58vrjxOW9EzIQ9FXusqVgXcDBWd/IZ3VUF1s5qvAF2/DDtoRUHZ0pOBLJMeqwd9u3/SU= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 May 2016 10:26:10.3732 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0801MB1306 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/06/2016 02:47 PM, Kuthonuzo Luruo wrote: > Currently, KASAN may fail to detect concurrent deallocations of the same > object due to a race in kasan_slab_free(). This patch makes double-free > detection more reliable by serializing access to KASAN object metadata. > New functions kasan_meta_lock() and kasan_meta_unlock() are provided to > lock/unlock per-object metadata. Double-free errors are now reported via > kasan_report(). > > Testing: > - Tested with a modified version of the 'slab_test' microbenchmark where > allocs occur on CPU 0; then all other CPUs concurrently attempt to free > the same object. > - Tested with new 'test_kasan' kasan_double_free() test in accompanying > patch. > > Signed-off-by: Kuthonuzo Luruo > --- > > Changes in v2: > - Incorporated suggestions from Dmitry Vyukov. New per-object metadata > lock/unlock functions; kasan_alloc_meta modified to add new state while > using fewer bits overall. > - Double-free pr_err promoted to kasan_report(). > - kasan_init_object() introduced to initialize KASAN object metadata > during slab creation. KASAN_STATE_INIT initialization removed from > kasan_poison_object_data(). > > --- > include/linux/kasan.h | 8 +++ > mm/kasan/kasan.c | 118 ++++++++++++++++++++++++++++++++++++------------- > mm/kasan/kasan.h | 15 +++++- > mm/kasan/quarantine.c | 7 +++- > mm/kasan/report.c | 31 +++++++++++-- > mm/slab.c | 1 + > 6 files changed, 142 insertions(+), 38 deletions(-) > Sorry, but this patch is crap. Something like this, will fix the race: --- mm/kasan/kasan.c | 20 ++++---------------- mm/kasan/kasan.h | 10 +++------- mm/kasan/quarantine.c | 1 - mm/kasan/report.c | 11 ++--------- 4 files changed, 9 insertions(+), 33 deletions(-) diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c index ef2e87b..8d078dc 100644 --- a/mm/kasan/kasan.c +++ b/mm/kasan/kasan.c @@ -419,13 +419,6 @@ void kasan_poison_object_data(struct kmem_cache *cache, void *object) kasan_poison_shadow(object, round_up(cache->object_size, KASAN_SHADOW_SCALE_SIZE), KASAN_KMALLOC_REDZONE); -#ifdef CONFIG_SLAB - if (cache->flags & SLAB_KASAN) { - struct kasan_alloc_meta *alloc_info = - get_alloc_info(cache, object); - alloc_info->state = KASAN_STATE_INIT; - } -#endif } #ifdef CONFIG_SLAB @@ -521,20 +514,15 @@ bool kasan_slab_free(struct kmem_cache *cache, void *object) struct kasan_free_meta *free_info = get_free_info(cache, object); - switch (alloc_info->state) { - case KASAN_STATE_ALLOC: - alloc_info->state = KASAN_STATE_QUARANTINE; + if (test_and_clear_bit(KASAN_STATE_ALLOCATED, + &alloc_info->state)) { quarantine_put(free_info, cache); set_track(&free_info->track, GFP_NOWAIT); kasan_poison_slab_free(cache, object); return true; - case KASAN_STATE_QUARANTINE: - case KASAN_STATE_FREE: + } else { pr_err("Double free"); dump_stack(); - break; - default: - break; } } return false; @@ -571,7 +559,7 @@ void kasan_kmalloc(struct kmem_cache *cache, const void *object, size_t size, struct kasan_alloc_meta *alloc_info = get_alloc_info(cache, object); - alloc_info->state = KASAN_STATE_ALLOC; + set_bit(KASAN_STATE_ALLOCATED, &alloc_info->state); alloc_info->alloc_size = size; set_track(&alloc_info->track, flags); } diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h index 7da78a6..2dcdc8f 100644 --- a/mm/kasan/kasan.h +++ b/mm/kasan/kasan.h @@ -60,10 +60,7 @@ struct kasan_global { */ enum kasan_state { - KASAN_STATE_INIT, - KASAN_STATE_ALLOC, - KASAN_STATE_QUARANTINE, - KASAN_STATE_FREE + KASAN_STATE_ALLOCATED, }; #define KASAN_STACK_DEPTH 64 @@ -75,9 +72,8 @@ struct kasan_track { struct kasan_alloc_meta { struct kasan_track track; - u32 state : 2; /* enum kasan_state */ - u32 alloc_size : 30; - u32 reserved; + unsigned long state; + u32 alloc_size; }; struct kasan_free_meta { diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c index 40159a6..ca33fd3 100644 --- a/mm/kasan/quarantine.c +++ b/mm/kasan/quarantine.c @@ -147,7 +147,6 @@ static void qlink_free(void **qlink, struct kmem_cache *cache) unsigned long flags; local_irq_save(flags); - alloc_info->state = KASAN_STATE_FREE; ___cache_free(cache, object, _THIS_IP_); local_irq_restore(flags); } diff --git a/mm/kasan/report.c b/mm/kasan/report.c index b3c122d..c2b0e51 100644 --- a/mm/kasan/report.c +++ b/mm/kasan/report.c @@ -140,18 +140,12 @@ static void object_err(struct kmem_cache *cache, struct page *page, pr_err("Object at %p, in cache %s\n", object, cache->name); if (!(cache->flags & SLAB_KASAN)) return; - switch (alloc_info->state) { - case KASAN_STATE_INIT: - pr_err("Object not allocated yet\n"); - break; - case KASAN_STATE_ALLOC: + if (test_bit(KASAN_STATE_ALLOCATED, &alloc_info->state)) { pr_err("Object allocated with size %u bytes.\n", alloc_info->alloc_size); pr_err("Allocation:\n"); print_track(&alloc_info->track); - break; - case KASAN_STATE_FREE: - case KASAN_STATE_QUARANTINE: + } else { pr_err("Object freed, allocated with size %u bytes\n", alloc_info->alloc_size); free_info = get_free_info(cache, object); @@ -159,7 +153,6 @@ static void object_err(struct kmem_cache *cache, struct page *page, print_track(&alloc_info->track); pr_err("Deallocation:\n"); print_track(&free_info->track); - break; } } #endif -- 2.7.3