Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752920AbdGMWkF (ORCPT ); Thu, 13 Jul 2017 18:40:05 -0400 Received: from mail-pg0-f49.google.com ([74.125.83.49]:33189 "EHLO mail-pg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751017AbdGMWkD (ORCPT ); Thu, 13 Jul 2017 18:40:03 -0400 Subject: Re: [PATCH 1/4] kasan: support alloca() poisoning To: Dmitry Vyukov Cc: Andrey Ryabinin , Alexander Potapenko , Masahiro Yamada , Michal Marek , LKML , kasan-dev , "linux-mm@kvack.org" , "open list:KERNEL BUILD + fi..." , Matthias Kaehlcke , Michael Davidson References: <20170706220114.142438-1-ghackmann@google.com> <20170706220114.142438-2-ghackmann@google.com> From: Greg Hackmann Message-ID: Date: Thu, 13 Jul 2017 15:40:00 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1931 Lines: 48 Hi, Thanks for taking a look at this patchstack. I apologize for the delay in responding. On 07/10/2017 01:44 AM, Dmitry Vyukov wrote: >> + >> + const void *left_redzone = (const void *)(addr - >> + KASAN_ALLOCA_REDZONE_SIZE); >> + const void *right_redzone = (const void *)(addr + rounded_up_size); > > Please check that size is rounded to KASAN_ALLOCA_REDZONE_SIZE. That's > the expectation, right? That can change is clang silently. > >> + kasan_poison_shadow(left_redzone, KASAN_ALLOCA_REDZONE_SIZE, >> + KASAN_ALLOCA_LEFT); >> + kasan_poison_shadow(right_redzone, >> + padding_size + KASAN_ALLOCA_REDZONE_SIZE, >> + KASAN_ALLOCA_RIGHT); > > We also need to poison the unaligned part at the end of the object > from size to rounded_up_size. You can see how we do it for heap > objects. The expectation is that `size' is the exact size of the alloca()ed object. `rounded_up_size' then adds the 0-7 bytes needed to adjust the size to the ASAN shadow scale. So `addr + rounded_up_size' should be the correct place to start poisoning. In retrospect this part of the code was pretty confusing. How about this? I think its intent is clearer, plus it's a closer match for the description in my commit message: unsigned long left_redzone_start; unsigned long object_end; unsigned long right_redzone_start, right_redzone_end; left_redzone_start = addr - KASAN_ALLOCA_REDZONE_SIZE; kasan_poison_shadow((const void *)left_redzone_start, KASAN_ALLOCA_REDZONE_SIZE, KASAN_ALLOCA_LEFT); object_end = round_up(addr + size, KASAN_SHADOW_SCALE_SIZE); right_redzone_start = round_up(object_end, KASAN_ALLOCA_REDZONE_SIZE); right_redzone_end = right_redzone_start + KASAN_ALLOCA_REDZONE_SIZE; kasan_poison_shadow((const void *)object_end, right_redzone_end - object_end, KASAN_ALLOCA_RIGHT);