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

Reply via email to