From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753037AbcEIMne (ORCPT ); Mon, 9 May 2016 08:43:34 -0400 Received: from mail-am1on0137.outbound.protection.outlook.com ([157.56.112.137]:15292 "EHLO emea01-am1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751764AbcEIMnc (ORCPT ); Mon, 9 May 2016 08:43:32 -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: Dmitry Vyukov References: <20160506114727.GA2571@cherokee.in.rdlabs.hpecorp.net> <573065BD.2020708@virtuozzo.com> CC: Kuthonuzo Luruo , Alexander Potapenko , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , kasan-dev , LKML , "linux-mm@kvack.org" From: Andrey Ryabinin Message-ID: <573085EB.5060808@virtuozzo.com> Date: Mon, 9 May 2016 15:43:23 +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: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [84.47.128.244] X-ClientProxiedBy: DB5PR02CA0031.eurprd02.prod.outlook.com (10.161.237.41) To HE1PR0801MB1307.eurprd08.prod.outlook.com (10.167.247.149) X-MS-Office365-Filtering-Correlation-Id: 254d7067-d555-4a47-3903-08d378078584 X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1307;2:7oBFBXnKm4KS3Q+xSFB1Y3Q5xcbESCz2SZh4vrWaD+yZ+ISCqTLfPEZoUZdVKGr16traNp0unegOGlppjDlQHuq+VNJ9J7/KT/VN65D0/nuUAiLxU51p09wPodPeGoXOMzd8ENsWiftG28Ay6TgniTg+gK+nntd36DhAiZyhvQ1axdNO47dtxj2iAmsdg34A;3:829CB9z2flB97k7JpqNLvnzzt6LXV4wjiGz82BaexNm/rkRBWuZ2ugZvwA9beYCoEJMQYE0He+j7OvHxa1ntGX+QAHOKz7TyC5KVNmw2+RgmHUYoYHLJLrSCtFcbDqRP X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0801MB1307; X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1307;25:WIwje3MakmfgC42GMZqJeKMNum08TO2eePXJ8t62Vbchvsw97G/6bPr8zwJnUzinLtufSOTB0rVe7ov/SGY2mNyh/nrEMuDB3K74ni/R42KHQnlW8qablgmEm0+sYnxX09PLf2BTLY6yNT/hJ8LJSkdiZbW7+WKjGNb7FzzdA2X9aMfQioQLhn2DIKDhK2cilKgY+3Leb1XutRauDk78YF9stfRMpFYS9BKw/q9D7hOKY0n0wR4rQ5CCc6wWxzNDCOJg2v/0aiLhKr4wVp1Cl3615jlTd6NL3qyapg+i495Z0FnJ8gD0Cte8YRejQqJx4fc99uj0FRaN3U3KcOCbSZ8TcgBS5qjH35S+FO3BYpYY7uQMsORgZLc4zGJWbRYUEDA3vftYw5I6gGHQQT0M9FOV/FcEfEmTs6YV8sDZVlvDvlTJ9stB1bon9ld09wOIEkex+0Rxi7fXeeqljjaqL0+vgQJSZCTOCyTPG8W4npq0LEShR3Gu6KVQqYKj9POXRwdRNXU5/A3FPHdhIXkNRL1jUDYFwJpZugl94iKbGp9/T4HmFUkIwNuXqVFcgfx1YS8+jExrCjCfDwjHi5i4VBqpwdBLc7g3KYiRgrUQZtNZmBKwuuS2rrOat7TGpidHEJ59B39f2sikza6t31mGRI1AXfaR/iPvpxYowL6NFXHD02yeX9Tio9mDHHcm2dyDZk9UaEcImrDQyU/zxXXAbzD9Yd8IO0gn/gKg2zJega/CAuZtAZ6k9nd0XN6CXA6egByfrxNZPB6rp11+4EZHwA== 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:HE1PR0801MB1307;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0801MB1307; X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1307;4:RQjciuw2QSxf7XaPdZ11Ah13InexOMxlbtuNYrxuE0TCpHL7gmqOlwVA4svGrr0SRvrBDyicgyFEe35uIoo/DlnA1DA5FCpB1BvpGdpMkF0WvXG4e+9B9S1dUddddiIwXa+gACVTiXqHhTYwlZik1Y9rct3DLLJk7/Z64ZMs+vm93I9HxV6hyuXANvzi0ae0l4wxXUNApsyYMidME28qdSyS+0qHRUlchuxmq3ujNJ0Fdn/QkJ8ZUuhjBBg9Vai4XflNWuRoyXg96BQXTV1paLJa4C+/mwmnyKo7+6CC5PNY5Ks2m6Ol4oiVaccyJdGjfKIubvU76XP4U1mB1/i7XUwIELM4JoemYuWn1ASHz8+ivT2/3IwyBLJWPeZr8WjwaKtaCj3jtlusHd6eX8JsPc8jjlYShAEk7j57oJCEdoY= X-Forefront-PRVS: 0937FB07C5 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6049001)(6009001)(377454003)(52314003)(24454002)(9170700001)(110136002)(575784001)(5004730100002)(86362001)(23676002)(42186005)(4001350100001)(189998001)(99136001)(65816999)(33656002)(50986999)(87266999)(76176999)(50466002)(54356999)(36756003)(47776003)(66066001)(65956001)(77096005)(586003)(5008740100001)(6116002)(3846002)(92566002)(230700001)(4326007)(2906002)(2950100001)(81166005)(19580395003)(19580405001);DIR:OUT;SFP:1102;SCL:1;SRVR:HE1PR0801MB1307;H:[10.52.82.2];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtIRTFQUjA4MDFNQjEzMDc7MjM6RnZEOWsxKzBlZ01sVUl3T216ZnVqWjRZ?= =?utf-8?B?Qk5rZXp5VFhzOWlPOUZ3NVJhWkRqYllGT2pXUmlmWE9oSUMzY2laZ0F4ZkhS?= =?utf-8?B?VnNoQmo0azRiRmk2QWlrYVNwZ012MWxNclVVMlBaUmR5b2N5QXlWdWt5Rmh5?= =?utf-8?B?TmhZWFJjUVFxRkdrQWM3V0UrTUZzd0xkWVM3L00xdzgvbDB6T25LcHFSMnRO?= =?utf-8?B?KzhBaGtCNU8zOVdRZTUyWUNZWGM4ZnhuRGZrMytDd3ROdWo2Mzh1UXd4QVdU?= =?utf-8?B?UVNLdFRjMHZFME15SlI2ZkcxWXlyMzBWRXJpQUtNbWFGS0FjN09IZTRJRUFu?= =?utf-8?B?Rmlob1htQnlKQ2VaNEIvblA3ODhPVXUrN0IyenB5ekJabEFIRGpkVEVkcSsx?= =?utf-8?B?NGI4K2x6K05JL0NUUVNONVpkQ0xFaEgzNng3RFVqbE5MNG5rR2JUL094Y3Rh?= =?utf-8?B?cFRQTUlYWW9VYVJrQS9YaDEwdGtaWWhzK2JSUXhkRklxV2cvRGZYdFBrT2lS?= =?utf-8?B?eHI2NFRYQWtuTzRkb2RhRG1EY0JncXF1ckhtc1hobFVyamRya1IydWVicm5q?= =?utf-8?B?cE5zcEZaaWJVREhDRW0ya3hybVZtL2E3Y0lpR1YvWEZaeUI1UlpFbWNqL1Vl?= =?utf-8?B?SFllZ2c5YVo2WEFOV05jSzJFWE5wWU84c2RVdU9iOWh6SjhuZnZhdEg3T1dG?= =?utf-8?B?S0JjdWtJODhrWHhqSng0S2g1RjVEYWlVMTNUVXkvQ3AwZk5UaVgwUzVwb25l?= =?utf-8?B?anBtUk16K01UeDNuT2Z4dDBCeDkxc0FFSkZDbFQ0TFNIVHRFNHZocEdCaXZB?= =?utf-8?B?d3I2Wld4NHk2elZzZExXWFIwTU45dWNvMExlYnY3K284ZWhWNDZscDlHT1dT?= =?utf-8?B?eXVqSndoSWhjLzNuZm5NWGJibEtuWitLL0hYTnVMaUtXZzlCVUpkWFluU0dZ?= =?utf-8?B?N0xyTWJwTEtGVVZEc1U0Qy91T2tOdkdHaHRFZ1liUXc5ek4yQ0JHQTcrdDBj?= =?utf-8?B?d29iMmRrTUFYdUMwQ0NPWS82amFXWWExZzNtUThnUitEYW5DVE1UYnc5bVhv?= =?utf-8?B?K1llek9rbGRmektFZnFkMnczeVlRbytVZHRkNE9KakJoUFhHb1pHVlpVTWlm?= =?utf-8?B?K00zNllRN01hOVpNQkJVSjN6Q2JJMFB2d2pKdDJxd0RhdUw2SWlrd08vcGF6?= =?utf-8?B?V1V2Nm8vNGR3M1hVYklyaXFmakhDcWNiMHc4R3F4Y2MwTzNUdEFONkRZVitP?= =?utf-8?B?TVBOYzI1NTRqSjlZN2dzOVhOZm9KSmQzU1pQeldPeWcwREIwbUgyQmpXYUFn?= =?utf-8?B?TERDd0JDL0Z4SmJ0N2UzbTVKVkhUOFdNazFpOWp5TVc2bTlGTlRUZHBUcWpq?= =?utf-8?B?dWNINUVmcUNPYWVOb1oxeXk3cE1xbnMxRmNJcDJQVzlQM1l3SktJWWZRNUNk?= =?utf-8?Q?H3zBRfWVR7FztZfYNWUHd450zRQQ6?= X-Microsoft-Exchange-Diagnostics: 1;HE1PR0801MB1307;5:v6MMKOL1tAoerMo+bINhHTOKltDTgHKgt1SKW0nn71RpMKwhYbo56Sr8Q3dh4KRU36roVLiBGqxOkBQW31tZ0tebDBDeIG79Om0fI7baXwXrLRPmwzW2s934hzc3ATn5WzZ2AZMYaS4Ifvj9oNjGrw==;24:b/4YFF4N9gMVOko7t2hWrqeaEPmw9y4sGJDUMKkScYZdh3rJ9EQ+6iTsPDfG9Er2jcUV4sLflKjdpPfn5dapswrlGUlT1l75g/0Ag05j3zE=;7:u7tBeM1jjfK4Rh126gJTdHs0+DffmKOkH12D0xPzjcAjikOZpxvkYUutgTQYICpIg1Ok2cxJkb1YBh5E4lJAbI6YoOMS/J7tSvvgd1MewQ4dv11Kbsr+rX4v/MGulVGDUeAlU7CJ7aRhZTGqMKk7nQrsFgq+DztfySDQZDr1HmtVvUlzsW2dnAVRLPzlAqqb;20:fBDoLDpCfINnoYPTpAJ9Zkwvld9jaznp++1VkXZvIwiUQW7ro2Vnv5vXs1PvNNQNnm4VaqNbcmfArJDRjV/5rl19YVIsZOKJ8Q8n5aHvKKDki+wiG5LLUvlY530oTQwlGSRQFQigA7BIBoiNAMo4EtnyLl01d4iEzw6eF5QDPZg= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 May 2016 12:43:27.3886 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0801MB1307 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/09/2016 01:31 PM, Dmitry Vyukov wrote: > On Mon, May 9, 2016 at 12:26 PM, Andrey Ryabinin > wrote: >> >> 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); > > alloc_info->track is not necessary initialized when > KASAN_STATE_ALLOCATED is set. It should be initialized to something. If it's not initialized, than that object wasn't allocated. So this would be a *very* random pointer access. Also this would mean that ->state itself might be not initialized too. Anyway, we can't do much in such scenario since we can't trust any data. And I don't think that we should because very likely this will cause panic eventually. > Worse, it can be initialized to a wrong > stack. > Define "wrong stack" here. I assume that you are talking about race in the following scenario: ------ Proccess A: Proccess B: p_A = kmalloc(); /* use p_A */ kfree(p_A); p_A = kmalloc(); .... set_bit(KASAN_STATE_ALLOCATED); //bit set, but stack is not saved yet. /* use after free p_A */ if (test_bit(KASAN_STATE_ALLOCATED)) print_stack() // will print track from Proccess A ----- So, would be the stack trace from A wrong in such situation? I don't think so. We could change ->state and save stack trace in the 'right' order with proper barriers, but would it prevent us from showing wrong stacktrace? - It wouldn't. Now, the only real problem with current code, is that we don't print free stack if we think that object is in allocated state. We should do this, because more information is always better, e.g. we might hit long-delayed use-after-free, in which case free stack would be useful (just like in scenario above).