* [PATCH v2 1/9] kasan: introduce helper functions for determining bug type
2017-03-02 13:48 [PATCH v2 0/9] kasan: improve error reports Andrey Konovalov
@ 2017-03-02 13:48 ` Andrey Konovalov
2017-03-02 17:19 ` Alexander Potapenko
2017-03-03 13:15 ` Andrey Ryabinin
2017-03-02 13:48 ` [PATCH v2 2/9] kasan: unify report headers Andrey Konovalov
` (8 subsequent siblings)
9 siblings, 2 replies; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-02 13:48 UTC (permalink / raw)
To: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel
Cc: Andrey Konovalov
Introduce get_shadow_bug_type() function, which determines bug type
based on the shadow value for a particular kernel address.
Introduce get_wild_bug_type() function, which determines bug type
for addresses which don't have a corresponding shadow value.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/kasan/report.c | 40 ++++++++++++++++++++++++++++++----------
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index f479365530b6..2790b4cadfa3 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -49,7 +49,13 @@ static const void *find_first_bad_addr(const void *addr, size_t size)
return first_bad_addr;
}
-static void print_error_description(struct kasan_access_info *info)
+static bool addr_has_shadow(struct kasan_access_info *info)
+{
+ return (info->access_addr >=
+ kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
+}
+
+static const char *get_shadow_bug_type(struct kasan_access_info *info)
{
const char *bug_type = "unknown-crash";
u8 *shadow_addr;
@@ -96,6 +102,27 @@ static void print_error_description(struct kasan_access_info *info)
break;
}
+ return bug_type;
+}
+
+const char *get_wild_bug_type(struct kasan_access_info *info)
+{
+ const char *bug_type = "unknown-crash";
+
+ if ((unsigned long)info->access_addr < PAGE_SIZE)
+ bug_type = "null-ptr-deref";
+ else if ((unsigned long)info->access_addr < TASK_SIZE)
+ bug_type = "user-memory-access";
+ else
+ bug_type = "wild-memory-access";
+
+ return bug_type;
+}
+
+static void print_error_description(struct kasan_access_info *info)
+{
+ const char *bug_type = get_shadow_bug_type(info);
+
pr_err("BUG: KASAN: %s in %pS at addr %p\n",
bug_type, (void *)info->ip,
info->access_addr);
@@ -265,18 +292,11 @@ static void print_shadow_for_address(const void *addr)
static void kasan_report_error(struct kasan_access_info *info)
{
unsigned long flags;
- const char *bug_type;
kasan_start_report(&flags);
- if (info->access_addr <
- kasan_shadow_to_mem((void *)KASAN_SHADOW_START)) {
- if ((unsigned long)info->access_addr < PAGE_SIZE)
- bug_type = "null-ptr-deref";
- else if ((unsigned long)info->access_addr < TASK_SIZE)
- bug_type = "user-memory-access";
- else
- bug_type = "wild-memory-access";
+ if (!addr_has_shadow(info)) {
+ const char *bug_type = get_wild_bug_type(info);
pr_err("BUG: KASAN: %s on address %p\n",
bug_type, info->access_addr);
pr_err("%s of size %zu by task %s/%d\n",
--
2.12.0.rc1.440.g5b76565f74-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 1/9] kasan: introduce helper functions for determining bug type
2017-03-02 13:48 ` [PATCH v2 1/9] kasan: introduce helper functions for determining bug type Andrey Konovalov
@ 2017-03-02 17:19 ` Alexander Potapenko
2017-03-03 13:15 ` Andrey Ryabinin
1 sibling, 0 replies; 28+ messages in thread
From: Alexander Potapenko @ 2017-03-02 17:19 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Andrey Ryabinin, Dmitry Vyukov, kasan-dev,
Linux Memory Management List, LKML
On Thu, Mar 2, 2017 at 2:48 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> Introduce get_shadow_bug_type() function, which determines bug type
> based on the shadow value for a particular kernel address.
> Introduce get_wild_bug_type() function, which determines bug type
> for addresses which don't have a corresponding shadow value.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
> mm/kasan/report.c | 40 ++++++++++++++++++++++++++++++----------
> 1 file changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index f479365530b6..2790b4cadfa3 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -49,7 +49,13 @@ static const void *find_first_bad_addr(const void *addr, size_t size)
> return first_bad_addr;
> }
>
> -static void print_error_description(struct kasan_access_info *info)
> +static bool addr_has_shadow(struct kasan_access_info *info)
> +{
> + return (info->access_addr >=
> + kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
> +}
> +
> +static const char *get_shadow_bug_type(struct kasan_access_info *info)
> {
> const char *bug_type = "unknown-crash";
> u8 *shadow_addr;
> @@ -96,6 +102,27 @@ static void print_error_description(struct kasan_access_info *info)
> break;
> }
>
> + return bug_type;
> +}
> +
> +const char *get_wild_bug_type(struct kasan_access_info *info)
> +{
> + const char *bug_type = "unknown-crash";
You don't seem to need "unknown-crash" here.
> + if ((unsigned long)info->access_addr < PAGE_SIZE)
> + bug_type = "null-ptr-deref";
> + else if ((unsigned long)info->access_addr < TASK_SIZE)
> + bug_type = "user-memory-access";
> + else
> + bug_type = "wild-memory-access";
> +
> + return bug_type;
> +}
> +
> +static void print_error_description(struct kasan_access_info *info)
> +{
> + const char *bug_type = get_shadow_bug_type(info);
> +
> pr_err("BUG: KASAN: %s in %pS at addr %p\n",
> bug_type, (void *)info->ip,
> info->access_addr);
> @@ -265,18 +292,11 @@ static void print_shadow_for_address(const void *addr)
> static void kasan_report_error(struct kasan_access_info *info)
> {
> unsigned long flags;
> - const char *bug_type;
>
> kasan_start_report(&flags);
>
> - if (info->access_addr <
> - kasan_shadow_to_mem((void *)KASAN_SHADOW_START)) {
> - if ((unsigned long)info->access_addr < PAGE_SIZE)
> - bug_type = "null-ptr-deref";
> - else if ((unsigned long)info->access_addr < TASK_SIZE)
> - bug_type = "user-memory-access";
> - else
> - bug_type = "wild-memory-access";
> + if (!addr_has_shadow(info)) {
> + const char *bug_type = get_wild_bug_type(info);
> pr_err("BUG: KASAN: %s on address %p\n",
> bug_type, info->access_addr);
> pr_err("%s of size %zu by task %s/%d\n",
> --
> 2.12.0.rc1.440.g5b76565f74-goog
>
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 1/9] kasan: introduce helper functions for determining bug type
2017-03-02 13:48 ` [PATCH v2 1/9] kasan: introduce helper functions for determining bug type Andrey Konovalov
2017-03-02 17:19 ` Alexander Potapenko
@ 2017-03-03 13:15 ` Andrey Ryabinin
1 sibling, 0 replies; 28+ messages in thread
From: Andrey Ryabinin @ 2017-03-03 13:15 UTC (permalink / raw)
To: Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel
On 03/02/2017 04:48 PM, Andrey Konovalov wrote:
> Introduce get_shadow_bug_type() function, which determines bug type
> based on the shadow value for a particular kernel address.
> Introduce get_wild_bug_type() function, which determines bug type
> for addresses which don't have a corresponding shadow value.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
> mm/kasan/report.c | 40 ++++++++++++++++++++++++++++++----------
> 1 file changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index f479365530b6..2790b4cadfa3 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -49,7 +49,13 @@ static const void *find_first_bad_addr(const void *addr, size_t size)
> return first_bad_addr;
> }
>
> -static void print_error_description(struct kasan_access_info *info)
> +static bool addr_has_shadow(struct kasan_access_info *info)
> +{
> + return (info->access_addr >=
> + kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
> +}
> +
> +static const char *get_shadow_bug_type(struct kasan_access_info *info)
> {
> const char *bug_type = "unknown-crash";
> u8 *shadow_addr;
> @@ -96,6 +102,27 @@ static void print_error_description(struct kasan_access_info *info)
> break;
> }
>
> + return bug_type;
> +}
> +
> +const char *get_wild_bug_type(struct kasan_access_info *info)
static
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 2/9] kasan: unify report headers
2017-03-02 13:48 [PATCH v2 0/9] kasan: improve error reports Andrey Konovalov
2017-03-02 13:48 ` [PATCH v2 1/9] kasan: introduce helper functions for determining bug type Andrey Konovalov
@ 2017-03-02 13:48 ` Andrey Konovalov
2017-03-02 13:48 ` [PATCH v2 3/9] kasan: change allocation and freeing stack traces headers Andrey Konovalov
` (7 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-02 13:48 UTC (permalink / raw)
To: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel
Cc: Andrey Konovalov
Unify KASAN report header format for different kinds of bad memory
accesses. Makes the code simpler.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/kasan/report.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 2790b4cadfa3..34a6d1aec524 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -119,16 +119,22 @@ const char *get_wild_bug_type(struct kasan_access_info *info)
return bug_type;
}
+static const char *get_bug_type(struct kasan_access_info *info)
+{
+ if (addr_has_shadow(info))
+ return get_shadow_bug_type(info);
+ return get_wild_bug_type(info);
+}
+
static void print_error_description(struct kasan_access_info *info)
{
- const char *bug_type = get_shadow_bug_type(info);
+ const char *bug_type = get_bug_type(info);
pr_err("BUG: KASAN: %s in %pS at addr %p\n",
- bug_type, (void *)info->ip,
- info->access_addr);
+ bug_type, (void *)info->ip, info->access_addr);
pr_err("%s of size %zu by task %s/%d\n",
- info->is_write ? "Write" : "Read",
- info->access_size, current->comm, task_pid_nr(current));
+ info->is_write ? "Write" : "Read", info->access_size,
+ current->comm, task_pid_nr(current));
}
static inline bool kernel_or_module_addr(const void *addr)
@@ -295,17 +301,11 @@ static void kasan_report_error(struct kasan_access_info *info)
kasan_start_report(&flags);
+ print_error_description(info);
+
if (!addr_has_shadow(info)) {
- const char *bug_type = get_wild_bug_type(info);
- pr_err("BUG: KASAN: %s on address %p\n",
- bug_type, info->access_addr);
- pr_err("%s of size %zu by task %s/%d\n",
- info->is_write ? "Write" : "Read",
- info->access_size, current->comm,
- task_pid_nr(current));
dump_stack();
} else {
- print_error_description(info);
print_address_description(info);
print_shadow_for_address(info->first_bad_addr);
}
--
2.12.0.rc1.440.g5b76565f74-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 3/9] kasan: change allocation and freeing stack traces headers
2017-03-02 13:48 [PATCH v2 0/9] kasan: improve error reports Andrey Konovalov
2017-03-02 13:48 ` [PATCH v2 1/9] kasan: introduce helper functions for determining bug type Andrey Konovalov
2017-03-02 13:48 ` [PATCH v2 2/9] kasan: unify report headers Andrey Konovalov
@ 2017-03-02 13:48 ` Andrey Konovalov
2017-03-02 13:48 ` [PATCH v2 4/9] kasan: simplify address description logic Andrey Konovalov
` (6 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-02 13:48 UTC (permalink / raw)
To: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel
Cc: Andrey Konovalov
Change stack traces headers from:
Allocated:
PID = 42
to:
Allocated by task 42:
Makes the report one line shorter and look better.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/kasan/report.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 34a6d1aec524..5922330237fd 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -175,9 +175,9 @@ static void kasan_end_report(unsigned long *flags)
kasan_enable_current();
}
-static void print_track(struct kasan_track *track)
+static void print_track(struct kasan_track *track, const char *prefix)
{
- pr_err("PID = %u\n", track->pid);
+ pr_err("%s by task %u:\n", prefix, track->pid);
if (track->stack) {
struct stack_trace trace;
@@ -199,10 +199,8 @@ static void kasan_object_err(struct kmem_cache *cache, void *object)
if (!(cache->flags & SLAB_KASAN))
return;
- pr_err("Allocated:\n");
- print_track(&alloc_info->alloc_track);
- pr_err("Freed:\n");
- print_track(&alloc_info->free_track);
+ print_track(&alloc_info->alloc_track, "Allocated");
+ print_track(&alloc_info->free_track, "Freed");
}
void kasan_report_double_free(struct kmem_cache *cache, void *object,
--
2.12.0.rc1.440.g5b76565f74-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 4/9] kasan: simplify address description logic
2017-03-02 13:48 [PATCH v2 0/9] kasan: improve error reports Andrey Konovalov
` (2 preceding siblings ...)
2017-03-02 13:48 ` [PATCH v2 3/9] kasan: change allocation and freeing stack traces headers Andrey Konovalov
@ 2017-03-02 13:48 ` Andrey Konovalov
2017-03-03 13:37 ` Andrey Ryabinin
2017-03-02 13:48 ` [PATCH v2 5/9] kasan: change report header Andrey Konovalov
` (5 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-02 13:48 UTC (permalink / raw)
To: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel
Cc: Andrey Konovalov
Simplify logic for describing a memory address.
Add addr_to_page() helper function.
Makes the code easier to follow.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/kasan/report.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 5922330237fd..8b0b27eb37cd 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -188,11 +188,18 @@ static void print_track(struct kasan_track *track, const char *prefix)
}
}
-static void kasan_object_err(struct kmem_cache *cache, void *object)
+static struct page *addr_to_page(const void *addr)
+{
+ if ((addr >= (void *)PAGE_OFFSET) &&
+ (addr < high_memory))
+ return virt_to_head_page(addr);
+ return NULL;
+}
+
+static void describe_object(struct kmem_cache *cache, void *object)
{
struct kasan_alloc_meta *alloc_info = get_alloc_info(cache, object);
- dump_stack();
pr_err("Object at %p, in cache %s size: %d\n", object, cache->name,
cache->object_size);
@@ -211,34 +218,32 @@ void kasan_report_double_free(struct kmem_cache *cache, void *object,
kasan_start_report(&flags);
pr_err("BUG: Double free or freeing an invalid pointer\n");
pr_err("Unexpected shadow byte: 0x%hhX\n", shadow);
- kasan_object_err(cache, object);
+ dump_stack();
+ describe_object(cache, object);
kasan_end_report(&flags);
}
static void print_address_description(struct kasan_access_info *info)
{
const void *addr = info->access_addr;
+ struct page *page = addr_to_page(addr);
- if ((addr >= (void *)PAGE_OFFSET) &&
- (addr < high_memory)) {
- struct page *page = virt_to_head_page(addr);
-
- if (PageSlab(page)) {
- void *object;
- struct kmem_cache *cache = page->slab_cache;
- object = nearest_obj(cache, page,
- (void *)info->access_addr);
- kasan_object_err(cache, object);
- return;
- }
+ if (page)
dump_page(page, "kasan: bad access detected");
+
+ dump_stack();
+
+ if (page && PageSlab(page)) {
+ struct kmem_cache *cache = page->slab_cache;
+ void *object = nearest_obj(cache, page, (void *)addr);
+
+ describe_object(cache, object);
}
if (kernel_or_module_addr(addr)) {
if (!init_task_stack_addr(addr))
pr_err("Address belongs to variable %pS\n", addr);
}
- dump_stack();
}
static bool row_is_guilty(const void *row, const void *guilty)
--
2.12.0.rc1.440.g5b76565f74-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 5/9] kasan: change report header
2017-03-02 13:48 [PATCH v2 0/9] kasan: improve error reports Andrey Konovalov
` (3 preceding siblings ...)
2017-03-02 13:48 ` [PATCH v2 4/9] kasan: simplify address description logic Andrey Konovalov
@ 2017-03-02 13:48 ` Andrey Konovalov
2017-03-03 13:21 ` Andrey Ryabinin
2017-03-02 13:48 ` [PATCH v2 6/9] kasan: improve slab object description Andrey Konovalov
` (4 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-02 13:48 UTC (permalink / raw)
To: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel
Cc: Andrey Konovalov
Change report header format from:
BUG: KASAN: use-after-free in unwind_get_return_address+0x28a/0x2c0 at addr ffff880069437950
Read of size 8 by task insmod/3925
to:
BUG: KASAN: use-after-free in unwind_get_return_address+0x28a/0x2c0
Read of size 8 at addr ffff880069437950 by task insmod/3925
The exact access address is not usually important, so move it to the
second line. This also makes the header look visually balanced.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/kasan/report.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 8b0b27eb37cd..945d0e13e8a4 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -130,11 +130,11 @@ static void print_error_description(struct kasan_access_info *info)
{
const char *bug_type = get_bug_type(info);
- pr_err("BUG: KASAN: %s in %pS at addr %p\n",
- bug_type, (void *)info->ip, info->access_addr);
- pr_err("%s of size %zu by task %s/%d\n",
+ pr_err("BUG: KASAN: %s in %pS\n",
+ bug_type, (void *)info->ip);
+ pr_err("%s of size %zu at addr %p by task %s/%d\n",
info->is_write ? "Write" : "Read", info->access_size,
- current->comm, task_pid_nr(current));
+ info->access_addr, current->comm, task_pid_nr(current));
}
static inline bool kernel_or_module_addr(const void *addr)
--
2.12.0.rc1.440.g5b76565f74-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 5/9] kasan: change report header
2017-03-02 13:48 ` [PATCH v2 5/9] kasan: change report header Andrey Konovalov
@ 2017-03-03 13:21 ` Andrey Ryabinin
2017-03-03 14:18 ` Andrey Konovalov
0 siblings, 1 reply; 28+ messages in thread
From: Andrey Ryabinin @ 2017-03-03 13:21 UTC (permalink / raw)
To: Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel
On 03/02/2017 04:48 PM, Andrey Konovalov wrote:
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index 8b0b27eb37cd..945d0e13e8a4 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -130,11 +130,11 @@ static void print_error_description(struct kasan_access_info *info)
> {
> const char *bug_type = get_bug_type(info);
>
> - pr_err("BUG: KASAN: %s in %pS at addr %p\n",
> - bug_type, (void *)info->ip, info->access_addr);
> - pr_err("%s of size %zu by task %s/%d\n",
> + pr_err("BUG: KASAN: %s in %pS\n",
> + bug_type, (void *)info->ip);
This should fit in one line without exceeding 80-char limit.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 5/9] kasan: change report header
2017-03-03 13:21 ` Andrey Ryabinin
@ 2017-03-03 14:18 ` Andrey Konovalov
2017-03-03 14:18 ` Andrey Konovalov
0 siblings, 1 reply; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-03 14:18 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: Alexander Potapenko, Dmitry Vyukov, kasan-dev, linux-mm, LKML
On Fri, Mar 3, 2017 at 2:21 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>
>
> On 03/02/2017 04:48 PM, Andrey Konovalov wrote:
>
>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>> index 8b0b27eb37cd..945d0e13e8a4 100644
>> --- a/mm/kasan/report.c
>> +++ b/mm/kasan/report.c
>> @@ -130,11 +130,11 @@ static void print_error_description(struct kasan_access_info *info)
>> {
>> const char *bug_type = get_bug_type(info);
>>
>> - pr_err("BUG: KASAN: %s in %pS at addr %p\n",
>> - bug_type, (void *)info->ip, info->access_addr);
>> - pr_err("%s of size %zu by task %s/%d\n",
>> + pr_err("BUG: KASAN: %s in %pS\n",
>> + bug_type, (void *)info->ip);
>
> This should fit in one line without exceeding 80-char limit.
You mean the code or the header?
The code fits, the header has much higher chances to fit after the change.
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to kasan-dev@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/028eee50-f14f-034d-6e8a-9d07276543b5%40virtuozzo.com.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 5/9] kasan: change report header
2017-03-03 14:18 ` Andrey Konovalov
@ 2017-03-03 14:18 ` Andrey Konovalov
0 siblings, 0 replies; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-03 14:18 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: Alexander Potapenko, Dmitry Vyukov, kasan-dev, linux-mm, LKML
On Fri, Mar 3, 2017 at 3:18 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> On Fri, Mar 3, 2017 at 2:21 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>
>>
>> On 03/02/2017 04:48 PM, Andrey Konovalov wrote:
>>
>>> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
>>> index 8b0b27eb37cd..945d0e13e8a4 100644
>>> --- a/mm/kasan/report.c
>>> +++ b/mm/kasan/report.c
>>> @@ -130,11 +130,11 @@ static void print_error_description(struct kasan_access_info *info)
>>> {
>>> const char *bug_type = get_bug_type(info);
>>>
>>> - pr_err("BUG: KASAN: %s in %pS at addr %p\n",
>>> - bug_type, (void *)info->ip, info->access_addr);
>>> - pr_err("%s of size %zu by task %s/%d\n",
>>> + pr_err("BUG: KASAN: %s in %pS\n",
>>> + bug_type, (void *)info->ip);
>>
>> This should fit in one line without exceeding 80-char limit.
>
> You mean the code or the header?
> The code fits, the header has much higher chances to fit after the change.
Ah, got you, will fix.
>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
>> To post to this group, send email to kasan-dev@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/028eee50-f14f-034d-6e8a-9d07276543b5%40virtuozzo.com.
>> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 6/9] kasan: improve slab object description
2017-03-02 13:48 [PATCH v2 0/9] kasan: improve error reports Andrey Konovalov
` (4 preceding siblings ...)
2017-03-02 13:48 ` [PATCH v2 5/9] kasan: change report header Andrey Konovalov
@ 2017-03-02 13:48 ` Andrey Konovalov
2017-03-03 13:31 ` Andrey Ryabinin
2017-03-02 13:48 ` [PATCH v2 7/9] kasan: print page description after stacks Andrey Konovalov
` (3 subsequent siblings)
9 siblings, 1 reply; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-02 13:48 UTC (permalink / raw)
To: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel
Cc: Andrey Konovalov
Changes slab object description from:
Object at ffff880068388540, in cache kmalloc-128 size: 128
to:
The buggy address belongs to the object at ffff880068388540
which belongs to the cache kmalloc-128 of size 128
The buggy address is located 123 bytes inside of
128-byte region [ffff880068388540, ffff8800683885c0)
Makes it more explanatory and adds information about relative offset
of the accessed address to the start of the object.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/kasan/report.c | 53 ++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 42 insertions(+), 11 deletions(-)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 945d0e13e8a4..8dfb7a060d69 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -196,18 +196,49 @@ static struct page *addr_to_page(const void *addr)
return NULL;
}
-static void describe_object(struct kmem_cache *cache, void *object)
+static void describe_object_addr(struct kmem_cache *cache, void *object,
+ const void *addr)
{
- struct kasan_alloc_meta *alloc_info = get_alloc_info(cache, object);
+ unsigned long access_addr = (unsigned long)addr;
+ unsigned long object_addr = (unsigned long)object;
+ const char *rel_type;
+ int rel_bytes;
- pr_err("Object at %p, in cache %s size: %d\n", object, cache->name,
- cache->object_size);
+ pr_err("The buggy address belongs to the object at %p\n"
+ " which belongs to the cache %s of size %d\n",
+ object, cache->name, cache->object_size);
- if (!(cache->flags & SLAB_KASAN))
+ if (!addr)
return;
- print_track(&alloc_info->alloc_track, "Allocated");
- print_track(&alloc_info->free_track, "Freed");
+ if (access_addr < object_addr) {
+ rel_type = "to the left";
+ rel_bytes = object_addr - access_addr;
+ } else if (access_addr >= object_addr + cache->object_size) {
+ rel_type = "to the right";
+ rel_bytes = access_addr - (object_addr + cache->object_size);
+ } else {
+ rel_type = "inside";
+ rel_bytes = access_addr - object_addr;
+ }
+
+ pr_err("The buggy address is located %d bytes %s of\n"
+ " %d-byte region [%p, %p)\n",
+ rel_bytes, rel_type, cache->object_size, (void *)object_addr,
+ (void *)(object_addr + cache->object_size));
+}
+
+static void describe_object(struct kmem_cache *cache, void *object,
+ const void *addr)
+{
+ struct kasan_alloc_meta *alloc_info = get_alloc_info(cache, object);
+
+ if (cache->flags & SLAB_KASAN) {
+ print_track(&alloc_info->alloc_track, "Allocated");
+ print_track(&alloc_info->free_track, "Freed");
+ }
+
+ describe_object_addr(cache, object, addr);
}
void kasan_report_double_free(struct kmem_cache *cache, void *object,
@@ -219,13 +250,13 @@ void kasan_report_double_free(struct kmem_cache *cache, void *object,
pr_err("BUG: Double free or freeing an invalid pointer\n");
pr_err("Unexpected shadow byte: 0x%hhX\n", shadow);
dump_stack();
- describe_object(cache, object);
+ describe_object(cache, object, NULL);
kasan_end_report(&flags);
}
static void print_address_description(struct kasan_access_info *info)
{
- const void *addr = info->access_addr;
+ void *addr = (void *)info->access_addr;
struct page *page = addr_to_page(addr);
if (page)
@@ -235,9 +266,9 @@ static void print_address_description(struct kasan_access_info *info)
if (page && PageSlab(page)) {
struct kmem_cache *cache = page->slab_cache;
- void *object = nearest_obj(cache, page, (void *)addr);
+ void *object = nearest_obj(cache, page, addr);
- describe_object(cache, object);
+ describe_object(cache, object, addr);
}
if (kernel_or_module_addr(addr)) {
--
2.12.0.rc1.440.g5b76565f74-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 6/9] kasan: improve slab object description
2017-03-02 13:48 ` [PATCH v2 6/9] kasan: improve slab object description Andrey Konovalov
@ 2017-03-03 13:31 ` Andrey Ryabinin
2017-03-03 13:52 ` Alexander Potapenko
0 siblings, 1 reply; 28+ messages in thread
From: Andrey Ryabinin @ 2017-03-03 13:31 UTC (permalink / raw)
To: Andrey Konovalov, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel
On 03/02/2017 04:48 PM, Andrey Konovalov wrote:
> Changes slab object description from:
>
> Object at ffff880068388540, in cache kmalloc-128 size: 128
>
> to:
>
> The buggy address belongs to the object at ffff880068388540
> which belongs to the cache kmalloc-128 of size 128
> The buggy address is located 123 bytes inside of
> 128-byte region [ffff880068388540, ffff8800683885c0)
>
> Makes it more explanatory and adds information about relative offset
> of the accessed address to the start of the object.
>
I don't think that this is an improvement. You replaced one simple line with a huge
and hard to parse text without giving any new/useful information.
Except maybe offset, it useful sometimes, so wouldn't mind adding it to description.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] kasan: improve slab object description
2017-03-03 13:31 ` Andrey Ryabinin
@ 2017-03-03 13:52 ` Alexander Potapenko
2017-03-03 14:39 ` Andrey Ryabinin
0 siblings, 1 reply; 28+ messages in thread
From: Alexander Potapenko @ 2017-03-03 13:52 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: Andrey Konovalov, Dmitry Vyukov, kasan-dev,
Linux Memory Management List, LKML
On Fri, Mar 3, 2017 at 2:31 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
> On 03/02/2017 04:48 PM, Andrey Konovalov wrote:
>> Changes slab object description from:
>>
>> Object at ffff880068388540, in cache kmalloc-128 size: 128
>>
>> to:
>>
>> The buggy address belongs to the object at ffff880068388540
>> which belongs to the cache kmalloc-128 of size 128
>> The buggy address is located 123 bytes inside of
>> 128-byte region [ffff880068388540, ffff8800683885c0)
>>
>> Makes it more explanatory and adds information about relative offset
>> of the accessed address to the start of the object.
>>
>
> I don't think that this is an improvement. You replaced one simple line with a huge
> and hard to parse text without giving any new/useful information.
> Except maybe offset, it useful sometimes, so wouldn't mind adding it to description.
Agreed.
How about:
===========
Access 123 bytes inside of 128-byte region [ffff880068388540, ffff8800683885c0)
Object at ffff880068388540 belongs to the cache kmalloc-128
===========
?
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to kasan-dev@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/db0b6605-32bc-4c7a-0c99-2e60e4bdb11f%40virtuozzo.com.
> For more options, visit https://groups.google.com/d/optout.
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] kasan: improve slab object description
2017-03-03 13:52 ` Alexander Potapenko
@ 2017-03-03 14:39 ` Andrey Ryabinin
2017-03-06 13:45 ` Andrey Konovalov
0 siblings, 1 reply; 28+ messages in thread
From: Andrey Ryabinin @ 2017-03-03 14:39 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Andrey Konovalov, Dmitry Vyukov, kasan-dev,
Linux Memory Management List, LKML
On 03/03/2017 04:52 PM, Alexander Potapenko wrote:
> On Fri, Mar 3, 2017 at 2:31 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>> On 03/02/2017 04:48 PM, Andrey Konovalov wrote:
>>> Changes slab object description from:
>>>
>>> Object at ffff880068388540, in cache kmalloc-128 size: 128
>>>
>>> to:
>>>
>>> The buggy address belongs to the object at ffff880068388540
>>> which belongs to the cache kmalloc-128 of size 128
>>> The buggy address is located 123 bytes inside of
>>> 128-byte region [ffff880068388540, ffff8800683885c0)
>>>
>>> Makes it more explanatory and adds information about relative offset
>>> of the accessed address to the start of the object.
>>>
>>
>> I don't think that this is an improvement. You replaced one simple line with a huge
>> and hard to parse text without giving any new/useful information.
>> Except maybe offset, it useful sometimes, so wouldn't mind adding it to description.
> Agreed.
> How about:
> ===========
> Access 123 bytes inside of 128-byte region [ffff880068388540, ffff8800683885c0)
> Object at ffff880068388540 belongs to the cache kmalloc-128
> ===========
> ?
>
I would just add the offset in the end:
Object at ffff880068388540, in cache kmalloc-128 size: 128 accessed at offset y
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] kasan: improve slab object description
2017-03-03 14:39 ` Andrey Ryabinin
@ 2017-03-06 13:45 ` Andrey Konovalov
2017-03-06 16:12 ` Andrey Ryabinin
0 siblings, 1 reply; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-06 13:45 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: Alexander Potapenko, Dmitry Vyukov, kasan-dev,
Linux Memory Management List, LKML
On Fri, Mar 3, 2017 at 3:39 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>
>
> On 03/03/2017 04:52 PM, Alexander Potapenko wrote:
>> On Fri, Mar 3, 2017 at 2:31 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>> On 03/02/2017 04:48 PM, Andrey Konovalov wrote:
>>>> Changes slab object description from:
>>>>
>>>> Object at ffff880068388540, in cache kmalloc-128 size: 128
>>>>
>>>> to:
>>>>
>>>> The buggy address belongs to the object at ffff880068388540
>>>> which belongs to the cache kmalloc-128 of size 128
>>>> The buggy address is located 123 bytes inside of
>>>> 128-byte region [ffff880068388540, ffff8800683885c0)
>>>>
>>>> Makes it more explanatory and adds information about relative offset
>>>> of the accessed address to the start of the object.
>>>>
>>>
>>> I don't think that this is an improvement. You replaced one simple line with a huge
>>> and hard to parse text without giving any new/useful information.
>>> Except maybe offset, it useful sometimes, so wouldn't mind adding it to description.
>> Agreed.
>> How about:
>> ===========
>> Access 123 bytes inside of 128-byte region [ffff880068388540, ffff8800683885c0)
>> Object at ffff880068388540 belongs to the cache kmalloc-128
>> ===========
>> ?
>>
>
> I would just add the offset in the end:
> Object at ffff880068388540, in cache kmalloc-128 size: 128 accessed at offset y
Access can be inside or outside the object, so it's better to
specifically say that.
I think we can do (basically what Alexander suggested):
Object at ffff880068388540 belongs to the cache kmalloc-128 of size 128
Access 123 bytes inside of 128-byte region [ffff880068388540, ffff8800683885c0)
What do you think?
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] kasan: improve slab object description
2017-03-06 13:45 ` Andrey Konovalov
@ 2017-03-06 16:12 ` Andrey Ryabinin
2017-03-06 17:05 ` Andrey Konovalov
0 siblings, 1 reply; 28+ messages in thread
From: Andrey Ryabinin @ 2017-03-06 16:12 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Alexander Potapenko, Dmitry Vyukov, kasan-dev,
Linux Memory Management List, LKML
On 03/06/2017 04:45 PM, Andrey Konovalov wrote:
> On Fri, Mar 3, 2017 at 3:39 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>
>>
>> On 03/03/2017 04:52 PM, Alexander Potapenko wrote:
>>> On Fri, Mar 3, 2017 at 2:31 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>>> On 03/02/2017 04:48 PM, Andrey Konovalov wrote:
>>>>> Changes slab object description from:
>>>>>
>>>>> Object at ffff880068388540, in cache kmalloc-128 size: 128
>>>>>
>>>>> to:
>>>>>
>>>>> The buggy address belongs to the object at ffff880068388540
>>>>> which belongs to the cache kmalloc-128 of size 128
>>>>> The buggy address is located 123 bytes inside of
>>>>> 128-byte region [ffff880068388540, ffff8800683885c0)
>>>>>
>>>>> Makes it more explanatory and adds information about relative offset
>>>>> of the accessed address to the start of the object.
>>>>>
>>>>
>>>> I don't think that this is an improvement. You replaced one simple line with a huge
>>>> and hard to parse text without giving any new/useful information.
>>>> Except maybe offset, it useful sometimes, so wouldn't mind adding it to description.
>>> Agreed.
>>> How about:
>>> ===========
>>> Access 123 bytes inside of 128-byte region [ffff880068388540, ffff8800683885c0)
>>> Object at ffff880068388540 belongs to the cache kmalloc-128
>>> ===========
>>> ?
>>>
>>
>> I would just add the offset in the end:
>> Object at ffff880068388540, in cache kmalloc-128 size: 128 accessed at offset y
>
> Access can be inside or outside the object, so it's better to
> specifically say that.
>
That what access offset and object's size tells us.
> I think we can do (basically what Alexander suggested):
>
> Object at ffff880068388540 belongs to the cache kmalloc-128 of size 128
> Access 123 bytes inside of 128-byte region [ffff880068388540, ffff8800683885c0)
This is just wrong and therefore very confusing. The message says that we access 123 bytes,
while in fact we access x-bytes at offset 123. IOW 123 sounds like access size here not the offset.
> What do you think?
>
Not better.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] kasan: improve slab object description
2017-03-06 16:12 ` Andrey Ryabinin
@ 2017-03-06 17:05 ` Andrey Konovalov
2017-03-06 17:16 ` Andrey Konovalov
0 siblings, 1 reply; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-06 17:05 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: Alexander Potapenko, Dmitry Vyukov, kasan-dev,
Linux Memory Management List, LKML
On Mon, Mar 6, 2017 at 5:12 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
> On 03/06/2017 04:45 PM, Andrey Konovalov wrote:
>> On Fri, Mar 3, 2017 at 3:39 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>>
>>>
>>> On 03/03/2017 04:52 PM, Alexander Potapenko wrote:
>>>> On Fri, Mar 3, 2017 at 2:31 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>>>> On 03/02/2017 04:48 PM, Andrey Konovalov wrote:
>>>>>> Changes slab object description from:
>>>>>>
>>>>>> Object at ffff880068388540, in cache kmalloc-128 size: 128
>>>>>>
>>>>>> to:
>>>>>>
>>>>>> The buggy address belongs to the object at ffff880068388540
>>>>>> which belongs to the cache kmalloc-128 of size 128
>>>>>> The buggy address is located 123 bytes inside of
>>>>>> 128-byte region [ffff880068388540, ffff8800683885c0)
>>>>>>
>>>>>> Makes it more explanatory and adds information about relative offset
>>>>>> of the accessed address to the start of the object.
>>>>>>
>>>>>
>>>>> I don't think that this is an improvement. You replaced one simple line with a huge
>>>>> and hard to parse text without giving any new/useful information.
>>>>> Except maybe offset, it useful sometimes, so wouldn't mind adding it to description.
>>>> Agreed.
>>>> How about:
>>>> ===========
>>>> Access 123 bytes inside of 128-byte region [ffff880068388540, ffff8800683885c0)
>>>> Object at ffff880068388540 belongs to the cache kmalloc-128
>>>> ===========
>>>> ?
>>>>
>>>
>>> I would just add the offset in the end:
>>> Object at ffff880068388540, in cache kmalloc-128 size: 128 accessed at offset y
>>
>> Access can be inside or outside the object, so it's better to
>> specifically say that.
>>
>
> That what access offset and object's size tells us.
>
>
>> I think we can do (basically what Alexander suggested):
>>
>> Object at ffff880068388540 belongs to the cache kmalloc-128 of size 128
>> Access 123 bytes inside of 128-byte region [ffff880068388540, ffff8800683885c0)
>
> This is just wrong and therefore very confusing. The message says that we access 123 bytes,
> while in fact we access x-bytes at offset 123. IOW 123 sounds like access size here not the offset.
What about
Object at ffff880068388540 belongs to cache kmalloc-128 of size 128
Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
?
>
>
>> What do you think?
>>
>
> Not better.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] kasan: improve slab object description
2017-03-06 17:05 ` Andrey Konovalov
@ 2017-03-06 17:16 ` Andrey Konovalov
2017-03-09 12:56 ` Andrey Ryabinin
0 siblings, 1 reply; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-06 17:16 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: Alexander Potapenko, Dmitry Vyukov, kasan-dev,
Linux Memory Management List, LKML
On Mon, Mar 6, 2017 at 6:05 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> On Mon, Mar 6, 2017 at 5:12 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>> On 03/06/2017 04:45 PM, Andrey Konovalov wrote:
>>> On Fri, Mar 3, 2017 at 3:39 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>>>
>>>>
>>>> On 03/03/2017 04:52 PM, Alexander Potapenko wrote:
>>>>> On Fri, Mar 3, 2017 at 2:31 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>>>>> On 03/02/2017 04:48 PM, Andrey Konovalov wrote:
>>>>>>> Changes slab object description from:
>>>>>>>
>>>>>>> Object at ffff880068388540, in cache kmalloc-128 size: 128
>>>>>>>
>>>>>>> to:
>>>>>>>
>>>>>>> The buggy address belongs to the object at ffff880068388540
>>>>>>> which belongs to the cache kmalloc-128 of size 128
>>>>>>> The buggy address is located 123 bytes inside of
>>>>>>> 128-byte region [ffff880068388540, ffff8800683885c0)
>>>>>>>
>>>>>>> Makes it more explanatory and adds information about relative offset
>>>>>>> of the accessed address to the start of the object.
>>>>>>>
>>>>>>
>>>>>> I don't think that this is an improvement. You replaced one simple line with a huge
>>>>>> and hard to parse text without giving any new/useful information.
>>>>>> Except maybe offset, it useful sometimes, so wouldn't mind adding it to description.
>>>>> Agreed.
>>>>> How about:
>>>>> ===========
>>>>> Access 123 bytes inside of 128-byte region [ffff880068388540, ffff8800683885c0)
>>>>> Object at ffff880068388540 belongs to the cache kmalloc-128
>>>>> ===========
>>>>> ?
>>>>>
>>>>
>>>> I would just add the offset in the end:
>>>> Object at ffff880068388540, in cache kmalloc-128 size: 128 accessed at offset y
>>>
>>> Access can be inside or outside the object, so it's better to
>>> specifically say that.
>>>
>>
>> That what access offset and object's size tells us.
>>
>>
>>> I think we can do (basically what Alexander suggested):
>>>
>>> Object at ffff880068388540 belongs to the cache kmalloc-128 of size 128
>>> Access 123 bytes inside of 128-byte region [ffff880068388540, ffff8800683885c0)
>>
>> This is just wrong and therefore very confusing. The message says that we access 123 bytes,
>> while in fact we access x-bytes at offset 123. IOW 123 sounds like access size here not the offset.
>
> What about
>
> Object at ffff880068388540 belongs to cache kmalloc-128 of size 128
> Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
>
> ?
Another alternative:
Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
Object belongs to cache kmalloc-128 of size 128
>
>>
>>
>>> What do you think?
>>>
>>
>> Not better.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] kasan: improve slab object description
2017-03-06 17:16 ` Andrey Konovalov
@ 2017-03-09 12:56 ` Andrey Ryabinin
2017-03-14 17:15 ` Andrey Konovalov
0 siblings, 1 reply; 28+ messages in thread
From: Andrey Ryabinin @ 2017-03-09 12:56 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Alexander Potapenko, Dmitry Vyukov, kasan-dev,
Linux Memory Management List, LKML
On 03/06/2017 08:16 PM, Andrey Konovalov wrote:
>>
>> What about
>>
>> Object at ffff880068388540 belongs to cache kmalloc-128 of size 128
>> Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
>>
>> ?
>
> Another alternative:
>
> Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
> Object belongs to cache kmalloc-128 of size 128
>
Is it something wrong with just printing offset at the end as I suggested earlier?
It's more compact and also more clear IMO.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] kasan: improve slab object description
2017-03-09 12:56 ` Andrey Ryabinin
@ 2017-03-14 17:15 ` Andrey Konovalov
2017-03-20 15:39 ` Andrey Ryabinin
0 siblings, 1 reply; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-14 17:15 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: Alexander Potapenko, Dmitry Vyukov, kasan-dev,
Linux Memory Management List, LKML
On Thu, Mar 9, 2017 at 1:56 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
> On 03/06/2017 08:16 PM, Andrey Konovalov wrote:
>
>>>
>>> What about
>>>
>>> Object at ffff880068388540 belongs to cache kmalloc-128 of size 128
>>> Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
>>>
>>> ?
>>
>> Another alternative:
>>
>> Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
>> Object belongs to cache kmalloc-128 of size 128
>>
>
> Is it something wrong with just printing offset at the end as I suggested earlier?
> It's more compact and also more clear IMO.
This is what you suggested:
Object at ffff880068388540, in cache kmalloc-128 size: 128 accessed at
offset 123
After minor reworking of punctuation, etc, we get:
Object at ffff880068388540, in cache kmalloc-128 of size 128, accessed
at offset 123
It's good, but I still don't like two things:
1. The line is quite long. Over 84 characters in this example, but
might be longer for different cache names. The solution would be to
split it into two lines.
2. The access might be within the object (for example use-after-free),
or outside the object (slab-out-of-bounds). In this case just saying
"accessed at offset X" might be confusing, since the offset might be
from the start of the object, or might be from the end. The solution
would be to specifically describe this.
Out of all options above this one I like the most:
>> Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
>> Object belongs to cache kmalloc-128 of size 128
as:
1. It specifies whether the offset is inside or outside the object.
2. The lines are not too long (the first one is 76 chars).
3. Accounts for larger cache names (the second line has some spare space).
4. Shows exact addresses of start and end of the object (it's possible
to calculate the end address using the start and the size, but it's
nicer to have it already calculated and shown).
Thanks!
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to kasan-dev@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/69679f30-e502-d2cf-8dee-4ee88f64f887%40virtuozzo.com.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 6/9] kasan: improve slab object description
2017-03-14 17:15 ` Andrey Konovalov
@ 2017-03-20 15:39 ` Andrey Ryabinin
2017-03-24 19:31 ` Andrey Konovalov
0 siblings, 1 reply; 28+ messages in thread
From: Andrey Ryabinin @ 2017-03-20 15:39 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Alexander Potapenko, Dmitry Vyukov, kasan-dev,
Linux Memory Management List, LKML
On 03/14/2017 08:15 PM, Andrey Konovalov wrote:
> On Thu, Mar 9, 2017 at 1:56 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>> On 03/06/2017 08:16 PM, Andrey Konovalov wrote:
>>
>>>>
>>>> What about
>>>>
>>>> Object at ffff880068388540 belongs to cache kmalloc-128 of size 128
>>>> Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
>>>>
>>>> ?
>>>
>>> Another alternative:
>>>
>>> Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
>>> Object belongs to cache kmalloc-128 of size 128
>>>
>>
>> Is it something wrong with just printing offset at the end as I suggested earlier?
>> It's more compact and also more clear IMO.
>
> This is what you suggested:
>
> Object at ffff880068388540, in cache kmalloc-128 size: 128 accessed at
> offset 123
>
> After minor reworking of punctuation, etc, we get:
>
> Object at ffff880068388540, in cache kmalloc-128 of size 128, accessed
> at offset 123
>
> It's good, but I still don't like two things:
>
> 1. The line is quite long. Over 84 characters in this example, but
> might be longer for different cache names. The solution would be to
> split it into two lines.
One line slightly larger than 80 chars is easier to read than
two IMO.
>
> 2. The access might be within the object (for example use-after-free),
> or outside the object (slab-out-of-bounds). In this case just saying
> "accessed at offset X" might be confusing, since the offset might be
> from the start of the object, or might be from the end. The solution
> would be to specifically describe this.
>
It's not confusing IMO.
It's pretty obvious that offset in the message "Object at <addr> ... accessed at offset <x>"
specifies the offset from the start of the object.
> Out of all options above this one I like the most:
>
>>> Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
>>> Object belongs to cache kmalloc-128 of size 128
>
> as:
>
> 1. It specifies whether the offset is inside or outside the object.
It doesn't really matter much whether is offset inside or outside.
Offset is only useful to identify what exactly struct/field accessed in situation like this:
x = a->b->c->d;
In other cases it usually just useless.
Also, note that you comparing access_addr against cache->object_size (which may be not equal to
the size requested by kmalloc)
+ if (access_addr < object_addr) {
+ rel_type = "to the left";
+ rel_bytes = object_addr - access_addr;
+ } else if (access_addr >= object_addr + cache->object_size) {
+ rel_type = "to the right";
+ rel_bytes = access_addr - (object_addr + cache->object_size);
+ } else {
+ rel_type = "inside";
+ rel_bytes = access_addr - object_addr;
+ }
+
So let's say we did kmalloc(100, GFP_KERNEL); This would mean that allocation
was from kmalloc-128 cache.
a) If we have off-by-one OOB access, we would see:
Accessed address is 100 bytes inside of [<start>, <start> + 128)
belongs to cache kmalloc-128 of size 128
b) And for the off-by-28 OOB, we would see:
Accessed address is 0 bytes to the right [<start>, <start> + 128)
belongs to cache kmalloc-128 of size 128
But I don't really see why we supposed to have different message for case a) b).
Comparing against requested size is possible only by looking into shadow. However that would
be complicated and also racy which means that you occasionally end up with some random numbers.
Also, I couldn't imagine why would anyone need to know the offset from the end of the object.
> 2. The lines are not too long (the first one is 76 chars).
> 3. Accounts for larger cache names (the second line has some spare space).
> 4. Shows exact addresses of start and end of the object (it's possible
> to calculate the end address using the start and the size, but it's
> nicer to have it already calculated and shown).
Come on we can do the simple math if needed.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 6/9] kasan: improve slab object description
2017-03-20 15:39 ` Andrey Ryabinin
@ 2017-03-24 19:31 ` Andrey Konovalov
0 siblings, 0 replies; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-24 19:31 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: Alexander Potapenko, Dmitry Vyukov, kasan-dev,
Linux Memory Management List, LKML
On Mon, Mar 20, 2017 at 4:39 PM, Andrey Ryabinin
<aryabinin@virtuozzo.com> wrote:
> On 03/14/2017 08:15 PM, Andrey Konovalov wrote:
>> On Thu, Mar 9, 2017 at 1:56 PM, Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
>>> On 03/06/2017 08:16 PM, Andrey Konovalov wrote:
>>>
>>>>>
>>>>> What about
>>>>>
>>>>> Object at ffff880068388540 belongs to cache kmalloc-128 of size 128
>>>>> Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
>>>>>
>>>>> ?
>>>>
>>>> Another alternative:
>>>>
>>>> Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
>>>> Object belongs to cache kmalloc-128 of size 128
>>>>
>>>
>>> Is it something wrong with just printing offset at the end as I suggested earlier?
>>> It's more compact and also more clear IMO.
>>
>> This is what you suggested:
>>
>> Object at ffff880068388540, in cache kmalloc-128 size: 128 accessed at
>> offset 123
>>
>> After minor reworking of punctuation, etc, we get:
>>
>> Object at ffff880068388540, in cache kmalloc-128 of size 128, accessed
>> at offset 123
>>
>> It's good, but I still don't like two things:
>>
>> 1. The line is quite long. Over 84 characters in this example, but
>> might be longer for different cache names. The solution would be to
>> split it into two lines.
>
> One line slightly larger than 80 chars is easier to read than
> two IMO.
>
>>
>> 2. The access might be within the object (for example use-after-free),
>> or outside the object (slab-out-of-bounds). In this case just saying
>> "accessed at offset X" might be confusing, since the offset might be
>> from the start of the object, or might be from the end. The solution
>> would be to specifically describe this.
>>
>
> It's not confusing IMO.
> It's pretty obvious that offset in the message "Object at <addr> ... accessed at offset <x>"
> specifies the offset from the start of the object.
>
>
>> Out of all options above this one I like the most:
>>
>>>> Accessed address is 123 bytes inside of [ffff880068388540, ffff8800683885c0)
>>>> Object belongs to cache kmalloc-128 of size 128
>>
>> as:
>>
>> 1. It specifies whether the offset is inside or outside the object.
>
> It doesn't really matter much whether is offset inside or outside.
> Offset is only useful to identify what exactly struct/field accessed in situation like this:
> x = a->b->c->d;
> In other cases it usually just useless.
>
> Also, note that you comparing access_addr against cache->object_size (which may be not equal to
> the size requested by kmalloc)
>
> + if (access_addr < object_addr) {
> + rel_type = "to the left";
> + rel_bytes = object_addr - access_addr;
> + } else if (access_addr >= object_addr + cache->object_size) {
> + rel_type = "to the right";
> + rel_bytes = access_addr - (object_addr + cache->object_size);
> + } else {
> + rel_type = "inside";
> + rel_bytes = access_addr - object_addr;
> + }
> +
>
> So let's say we did kmalloc(100, GFP_KERNEL); This would mean that allocation
> was from kmalloc-128 cache.
>
> a) If we have off-by-one OOB access, we would see:
> Accessed address is 100 bytes inside of [<start>, <start> + 128)
> belongs to cache kmalloc-128 of size 128
>
> b) And for the off-by-28 OOB, we would see:
> Accessed address is 0 bytes to the right [<start>, <start> + 128)
> belongs to cache kmalloc-128 of size 128
>
> But I don't really see why we supposed to have different message for case a) b).
>
> Comparing against requested size is possible only by looking into shadow. However that would
> be complicated and also racy which means that you occasionally end up with some random numbers.
OK, makes sense.
I hope you don't mind if I put the offset at the next line.
>
> Also, I couldn't imagine why would anyone need to know the offset from the end of the object.
>
>> 2. The lines are not too long (the first one is 76 chars).
>> 3. Accounts for larger cache names (the second line has some spare space).
>> 4. Shows exact addresses of start and end of the object (it's possible
>> to calculate the end address using the start and the size, but it's
>> nicer to have it already calculated and shown).
>
> Come on we can do the simple math if needed.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 7/9] kasan: print page description after stacks
2017-03-02 13:48 [PATCH v2 0/9] kasan: improve error reports Andrey Konovalov
` (5 preceding siblings ...)
2017-03-02 13:48 ` [PATCH v2 6/9] kasan: improve slab object description Andrey Konovalov
@ 2017-03-02 13:48 ` Andrey Konovalov
2017-03-02 13:48 ` [PATCH v2 8/9] kasan: improve double-free report format Andrey Konovalov
` (2 subsequent siblings)
9 siblings, 0 replies; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-02 13:48 UTC (permalink / raw)
To: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel
Cc: Andrey Konovalov
Moves page description after the stacks since it's less important.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/kasan/report.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 8dfb7a060d69..1d2b15174a98 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -259,9 +259,6 @@ static void print_address_description(struct kasan_access_info *info)
void *addr = (void *)info->access_addr;
struct page *page = addr_to_page(addr);
- if (page)
- dump_page(page, "kasan: bad access detected");
-
dump_stack();
if (page && PageSlab(page)) {
@@ -271,9 +268,14 @@ static void print_address_description(struct kasan_access_info *info)
describe_object(cache, object, addr);
}
- if (kernel_or_module_addr(addr)) {
- if (!init_task_stack_addr(addr))
- pr_err("Address belongs to variable %pS\n", addr);
+ if (kernel_or_module_addr(addr) && !init_task_stack_addr(addr)) {
+ pr_err("The buggy address belongs to the variable:\n");
+ pr_err(" %pS\n", addr);
+ }
+
+ if (page) {
+ pr_err("The buggy address belongs to the page:\n");
+ dump_page(page, "kasan: bad access detected");
}
}
--
2.12.0.rc1.440.g5b76565f74-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 8/9] kasan: improve double-free report format
2017-03-02 13:48 [PATCH v2 0/9] kasan: improve error reports Andrey Konovalov
` (6 preceding siblings ...)
2017-03-02 13:48 ` [PATCH v2 7/9] kasan: print page description after stacks Andrey Konovalov
@ 2017-03-02 13:48 ` Andrey Konovalov
2017-03-02 13:48 ` [PATCH v2 9/9] kasan: separate report parts by empty lines Andrey Konovalov
2017-03-02 13:57 ` [PATCH v2 0/9] kasan: improve error reports Dmitry Vyukov
9 siblings, 0 replies; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-02 13:48 UTC (permalink / raw)
To: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel
Cc: Andrey Konovalov
Changes double-free report header from:
BUG: Double free or freeing an invalid pointer
Unexpected shadow byte: 0xFB
to:
BUG: KASAN: double-free or invalid-free in kmalloc_oob_left+0xe5/0xef
This makes a bug uniquely identifiable by the first report line.
To account for removing of the unexpected shadow value, print shadow
bytes at the end of the report as in reports for other kinds of bugs.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/kasan/kasan.c | 3 ++-
mm/kasan/kasan.h | 2 +-
mm/kasan/report.c | 30 ++++++++++++++----------------
3 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index 25f0e6521f36..bd89fe06bb86 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -566,7 +566,8 @@ bool kasan_slab_free(struct kmem_cache *cache, void *object)
shadow_byte = READ_ONCE(*(s8 *)kasan_mem_to_shadow(object));
if (shadow_byte < 0 || shadow_byte >= KASAN_SHADOW_SCALE_SIZE) {
- kasan_report_double_free(cache, object, shadow_byte);
+ kasan_report_double_free(cache, object,
+ __builtin_return_address(1));
return true;
}
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 1c260e6b3b3c..75729173ade9 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -104,7 +104,7 @@ static inline bool kasan_report_enabled(void)
void kasan_report(unsigned long addr, size_t size,
bool is_write, unsigned long ip);
void kasan_report_double_free(struct kmem_cache *cache, void *object,
- s8 shadow);
+ void *ip);
#if defined(CONFIG_SLAB) || defined(CONFIG_SLUB)
void quarantine_put(struct kasan_free_meta *info, struct kmem_cache *cache);
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 1d2b15174a98..210131bc0a3c 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -241,22 +241,8 @@ static void describe_object(struct kmem_cache *cache, void *object,
describe_object_addr(cache, object, addr);
}
-void kasan_report_double_free(struct kmem_cache *cache, void *object,
- s8 shadow)
-{
- unsigned long flags;
-
- kasan_start_report(&flags);
- pr_err("BUG: Double free or freeing an invalid pointer\n");
- pr_err("Unexpected shadow byte: 0x%hhX\n", shadow);
- dump_stack();
- describe_object(cache, object, NULL);
- kasan_end_report(&flags);
-}
-
-static void print_address_description(struct kasan_access_info *info)
+static void print_address_description(void *addr)
{
- void *addr = (void *)info->access_addr;
struct page *page = addr_to_page(addr);
dump_stack();
@@ -331,6 +317,18 @@ static void print_shadow_for_address(const void *addr)
}
}
+void kasan_report_double_free(struct kmem_cache *cache, void *object,
+ void *ip)
+{
+ unsigned long flags;
+
+ kasan_start_report(&flags);
+ pr_err("BUG: KASAN: double-free or invalid-free in %pS\n", ip);
+ print_address_description(object);
+ print_shadow_for_address(object);
+ kasan_end_report(&flags);
+}
+
static void kasan_report_error(struct kasan_access_info *info)
{
unsigned long flags;
@@ -342,7 +340,7 @@ static void kasan_report_error(struct kasan_access_info *info)
if (!addr_has_shadow(info)) {
dump_stack();
} else {
- print_address_description(info);
+ print_address_description((void *)info->access_addr);
print_shadow_for_address(info->first_bad_addr);
}
--
2.12.0.rc1.440.g5b76565f74-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v2 9/9] kasan: separate report parts by empty lines
2017-03-02 13:48 [PATCH v2 0/9] kasan: improve error reports Andrey Konovalov
` (7 preceding siblings ...)
2017-03-02 13:48 ` [PATCH v2 8/9] kasan: improve double-free report format Andrey Konovalov
@ 2017-03-02 13:48 ` Andrey Konovalov
2017-03-02 13:57 ` [PATCH v2 0/9] kasan: improve error reports Dmitry Vyukov
9 siblings, 0 replies; 28+ messages in thread
From: Andrey Konovalov @ 2017-03-02 13:48 UTC (permalink / raw)
To: Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, kasan-dev,
linux-mm, linux-kernel
Cc: Andrey Konovalov
Makes the report easier to read.
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
mm/kasan/report.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 210131bc0a3c..718a10a48a19 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -235,7 +235,9 @@ static void describe_object(struct kmem_cache *cache, void *object,
if (cache->flags & SLAB_KASAN) {
print_track(&alloc_info->alloc_track, "Allocated");
+ pr_err("\n");
print_track(&alloc_info->free_track, "Freed");
+ pr_err("\n");
}
describe_object_addr(cache, object, addr);
@@ -246,6 +248,7 @@ static void print_address_description(void *addr)
struct page *page = addr_to_page(addr);
dump_stack();
+ pr_err("\n");
if (page && PageSlab(page)) {
struct kmem_cache *cache = page->slab_cache;
@@ -324,7 +327,9 @@ void kasan_report_double_free(struct kmem_cache *cache, void *object,
kasan_start_report(&flags);
pr_err("BUG: KASAN: double-free or invalid-free in %pS\n", ip);
+ pr_err("\n");
print_address_description(object);
+ pr_err("\n");
print_shadow_for_address(object);
kasan_end_report(&flags);
}
@@ -336,11 +341,13 @@ static void kasan_report_error(struct kasan_access_info *info)
kasan_start_report(&flags);
print_error_description(info);
+ pr_err("\n");
if (!addr_has_shadow(info)) {
dump_stack();
} else {
print_address_description((void *)info->access_addr);
+ pr_err("\n");
print_shadow_for_address(info->first_bad_addr);
}
--
2.12.0.rc1.440.g5b76565f74-goog
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v2 0/9] kasan: improve error reports
2017-03-02 13:48 [PATCH v2 0/9] kasan: improve error reports Andrey Konovalov
` (8 preceding siblings ...)
2017-03-02 13:48 ` [PATCH v2 9/9] kasan: separate report parts by empty lines Andrey Konovalov
@ 2017-03-02 13:57 ` Dmitry Vyukov
9 siblings, 0 replies; 28+ messages in thread
From: Dmitry Vyukov @ 2017-03-02 13:57 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Andrey Ryabinin, Alexander Potapenko, kasan-dev, linux-mm, LKML,
Andrew Morton
On Thu, Mar 2, 2017 at 2:48 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> This patchset improves KASAN reports by making them easier to read
> and a little more detailed.
> Also improves mm/kasan/report.c readability.
Acked-by: Dmitry Vyukov <dvyukov@google.com>
> Effectively changes a use-after-free report to:
>
> ==================================================================
> BUG: KASAN: use-after-free in kmalloc_uaf+0xaa/0xb6 [test_kasan]
> Write of size 1 at addr ffff88006aa59da8 by task insmod/3951
>
> CPU: 1 PID: 3951 Comm: insmod Tainted: G B 4.10.0+ #84
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Call Trace:
> dump_stack+0x292/0x398
> print_address_description+0x73/0x280
> kasan_report.part.2+0x207/0x2f0
> __asan_report_store1_noabort+0x2c/0x30
> kmalloc_uaf+0xaa/0xb6 [test_kasan]
> kmalloc_tests_init+0x4f/0xa48 [test_kasan]
> do_one_initcall+0xf3/0x390
> do_init_module+0x215/0x5d0
> load_module+0x54de/0x82b0
> SYSC_init_module+0x3be/0x430
> SyS_init_module+0x9/0x10
> entry_SYSCALL_64_fastpath+0x1f/0xc2
> RIP: 0033:0x7f22cfd0b9da
> RSP: 002b:00007ffe69118a78 EFLAGS: 00000206 ORIG_RAX: 00000000000000af
> RAX: ffffffffffffffda RBX: 0000555671242090 RCX: 00007f22cfd0b9da
> RDX: 00007f22cffcaf88 RSI: 000000000004df7e RDI: 00007f22d0399000
> RBP: 00007f22cffcaf88 R08: 0000000000000003 R09: 0000000000000000
> R10: 00007f22cfd07d0a R11: 0000000000000206 R12: 0000555671243190
> R13: 000000000001fe81 R14: 0000000000000000 R15: 0000000000000004
>
> Allocated by task 3951:
> save_stack_trace+0x16/0x20
> save_stack+0x43/0xd0
> kasan_kmalloc+0xad/0xe0
> kmem_cache_alloc_trace+0x82/0x270
> kmalloc_uaf+0x56/0xb6 [test_kasan]
> kmalloc_tests_init+0x4f/0xa48 [test_kasan]
> do_one_initcall+0xf3/0x390
> do_init_module+0x215/0x5d0
> load_module+0x54de/0x82b0
> SYSC_init_module+0x3be/0x430
> SyS_init_module+0x9/0x10
> entry_SYSCALL_64_fastpath+0x1f/0xc2
>
> Freed by task 3951:
> save_stack_trace+0x16/0x20
> save_stack+0x43/0xd0
> kasan_slab_free+0x72/0xc0
> kfree+0xe8/0x2b0
> kmalloc_uaf+0x85/0xb6 [test_kasan]
> kmalloc_tests_init+0x4f/0xa48 [test_kasan]
> do_one_initcall+0xf3/0x390
> do_init_module+0x215/0x5d0
> load_module+0x54de/0x82b0
> SYSC_init_module+0x3be/0x430
> SyS_init_module+0x9/0x10
> entry_SYSCALL_64_fastpath+0x1f/0xc
>
> The buggy address belongs to the object at ffff88006aa59da0
> which belongs to the cache kmalloc-16 of size 16
> The buggy address is located 8 bytes inside of
> 16-byte region [ffff88006aa59da0, ffff88006aa59db0)
> The buggy address belongs to the page:
> page:ffffea0001aa9640 count:1 mapcount:0 mapping: (null) index:0x0
> flags: 0x100000000000100(slab)
> raw: 0100000000000100 0000000000000000 0000000000000000 0000000180800080
> raw: ffffea0001abe380 0000000700000007 ffff88006c401b40 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff88006aa59c80: 00 00 fc fc 00 00 fc fc 00 00 fc fc 00 00 fc fc
> ffff88006aa59d00: 00 00 fc fc 00 00 fc fc 00 00 fc fc 00 00 fc fc
>>ffff88006aa59d80: fb fb fc fc fb fb fc fc fb fb fc fc fb fb fc fc
> ^
> ffff88006aa59e00: fb fb fc fc fb fb fc fc fb fb fc fc fb fb fc fc
> ffff88006aa59e80: fb fb fc fc 00 00 fc fc 00 00 fc fc 00 00 fc fc
> ==================================================================
>
> from:
>
> ==================================================================
> BUG: KASAN: use-after-free in kmalloc_uaf+0xaa/0xb6 [test_kasan] at addr ffff88006c4dcb28
> Write of size 1 by task insmod/3984
> CPU: 1 PID: 3984 Comm: insmod Tainted: G B 4.10.0+ #83
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Call Trace:
> dump_stack+0x292/0x398
> kasan_object_err+0x1c/0x70
> kasan_report.part.1+0x20e/0x4e0
> __asan_report_store1_noabort+0x2c/0x30
> kmalloc_uaf+0xaa/0xb6 [test_kasan]
> kmalloc_tests_init+0x4f/0xa48 [test_kasan]
> do_one_initcall+0xf3/0x390
> do_init_module+0x215/0x5d0
> load_module+0x54de/0x82b0
> SYSC_init_module+0x3be/0x430
> SyS_init_module+0x9/0x10
> entry_SYSCALL_64_fastpath+0x1f/0xc2
> RIP: 0033:0x7feca0f779da
> RSP: 002b:00007ffdfeae5218 EFLAGS: 00000206 ORIG_RAX: 00000000000000af
> RAX: ffffffffffffffda RBX: 000055a064c13090 RCX: 00007feca0f779da
> RDX: 00007feca1236f88 RSI: 000000000004df7e RDI: 00007feca1605000
> RBP: 00007feca1236f88 R08: 0000000000000003 R09: 0000000000000000
> R10: 00007feca0f73d0a R11: 0000000000000206 R12: 000055a064c14190
> R13: 000000000001fe81 R14: 0000000000000000 R15: 0000000000000004
> Object at ffff88006c4dcb20, in cache kmalloc-16 size: 16
> Allocated:
> PID = 3984
> save_stack_trace+0x16/0x20
> save_stack+0x43/0xd0
> kasan_kmalloc+0xad/0xe0
> kmem_cache_alloc_trace+0x82/0x270
> kmalloc_uaf+0x56/0xb6 [test_kasan]
> kmalloc_tests_init+0x4f/0xa48 [test_kasan]
> do_one_initcall+0xf3/0x390
> do_init_module+0x215/0x5d0
> load_module+0x54de/0x82b0
> SYSC_init_module+0x3be/0x430
> SyS_init_module+0x9/0x10
> entry_SYSCALL_64_fastpath+0x1f/0xc2
> Freed:
> PID = 3984
> save_stack_trace+0x16/0x20
> save_stack+0x43/0xd0
> kasan_slab_free+0x73/0xc0
> kfree+0xe8/0x2b0
> kmalloc_uaf+0x85/0xb6 [test_kasan]
> kmalloc_tests_init+0x4f/0xa48 [test_kasan]
> do_one_initcall+0xf3/0x390
> do_init_module+0x215/0x5d0
> load_module+0x54de/0x82b0
> SYSC_init_module+0x3be/0x430
> SyS_init_module+0x9/0x10
> entry_SYSCALL_64_fastpath+0x1f/0xc2
> Memory state around the buggy address:
> ffff88006c4dca00: fb fb fc fc fb fb fc fc fb fb fc fc fb fb fc fc
> ffff88006c4dca80: fb fb fc fc fb fb fc fc fb fb fc fc fb fb fc fc
>>ffff88006c4dcb00: fb fb fc fc fb fb fc fc fb fb fc fc fb fb fc fc
> ^
> ffff88006c4dcb80: fb fb fc fc 00 00 fc fc fb fb fc fc fb fb fc fc
> ffff88006c4dcc00: fb fb fc fc fb fb fc fc fb fb fc fc fb fb fc fc
> ==================================================================
>
> Changes in v2:
> - split patch in multiple smaller ones
> - improve double-free reports
>
> Andrey Konovalov (9):
> kasan: introduce helper functions for determining bug type
> kasan: unify report headers
> kasan: change allocation and freeing stack traces headers
> kasan: simplify address description logic
> kasan: change report header
> kasan: improve slab object description
> kasan: print page description after stacks
> kasan: improve double-free report format
> kasan: separate report parts by empty lines
>
> mm/kasan/kasan.c | 3 +-
> mm/kasan/kasan.h | 2 +-
> mm/kasan/report.c | 187 ++++++++++++++++++++++++++++++++++++------------------
> 3 files changed, 127 insertions(+), 65 deletions(-)
>
> --
> 2.12.0.rc1.440.g5b76565f74-goog
>
^ permalink raw reply [flat|nested] 28+ messages in thread