linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Greg Hackmann <ghackmann@google.com>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Alexander Potapenko <glider@google.com>,
	Masahiro Yamada <yamada.masahiro@socionext.com>,
	Michal Marek <mmarek@suse.com>,
	LKML <linux-kernel@vger.kernel.org>,
	kasan-dev <kasan-dev@googlegroups.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"open list:KERNEL BUILD + fi..." <linux-kbuild@vger.kernel.org>,
	Matthias Kaehlcke <mka@chromium.org>,
	Michael Davidson <md@google.com>
Subject: Re: [PATCH 1/4] kasan: support alloca() poisoning
Date: Thu, 13 Jul 2017 15:40:00 -0700	[thread overview]
Message-ID: <c7160aca-a203-e3d8-eb49-b051aff78f0e@google.com> (raw)
In-Reply-To: <CACT4Y+YWLc3n-PBcD1Cmu_FLGSDd+vyTTyeBamk2bBZhdWJSoA@mail.gmail.com>

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);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2017-07-13 22:40 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-06 22:01 [PATCH 0/4] kasan: add clang support Greg Hackmann
2017-07-06 22:01 ` [PATCH 1/4] kasan: support alloca() poisoning Greg Hackmann
2017-07-07  0:09   ` Greg Hackmann
2017-07-10  8:44   ` Dmitry Vyukov
2017-07-13 22:40     ` Greg Hackmann [this message]
2017-07-14  6:13       ` Dmitry Vyukov
2017-07-10 10:30   ` Andrey Ryabinin
2017-07-13 22:49     ` Greg Hackmann
2017-07-14 16:52       ` Andrey Ryabinin
2017-07-06 22:01 ` [PATCH 2/4] kasan: added functions for unpoisoning stack variables Greg Hackmann
2017-07-10  8:46   ` Dmitry Vyukov
2017-07-10 10:31   ` Andrey Ryabinin
2017-07-06 22:01 ` [PATCH 3/4] kasan: support LLVM-style asan parameters Greg Hackmann
2017-07-10  8:47   ` Dmitry Vyukov
2017-07-06 22:01 ` [PATCH 4/4] kasan: add compiler support for clang Greg Hackmann
2017-07-10  8:48   ` Dmitry Vyukov
2017-07-10 10:34   ` Andrey Ryabinin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c7160aca-a203-e3d8-eb49-b051aff78f0e@google.com \
    --to=ghackmann@google.com \
    --cc=aryabinin@virtuozzo.com \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=md@google.com \
    --cc=mka@chromium.org \
    --cc=mmarek@suse.com \
    --cc=yamada.masahiro@socionext.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox