Re: [PATCH v3 2/3] kasan: integrate the common part of two KASAN tag-based modes

From: Kuan-Ying Lee
Date: Tue Jun 22 2021 - 23:18:08 EST


On Tue, 2021-06-22 at 16:54 +0300, Andrey Konovalov wrote:
> On Sun, Jun 20, 2021 at 2:48 PM Kuan-Ying Lee
> <Kuan-Ying.Lee@xxxxxxxxxxxx> wrote:
> >
> > 1. Move kasan_get_free_track() and kasan_set_free_info()
> > into tags.c
>
> Please mention that the patch doesn't only move but also combines
> these functions for SW_TAGS and HW_TAGS modes.
>

Got it.

> > --- /dev/null
> > +++ b/mm/kasan/report_tags.h
> > @@ -0,0 +1,55 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> > + * Copyright (c) 2020 Google, Inc.
> > + */
> > +#ifndef __MM_KASAN_REPORT_TAGS_H
> > +#define __MM_KASAN_REPORT_TAGS_H
> > +
> > +#include "kasan.h"
> > +#include "../slab.h"
> > +
> > +const char *kasan_get_bug_type(struct kasan_access_info *info)
>
> As mentioned by Alex, don't put this implementation into a header.
> Put
> it into report_tags.c. The declaration is already in kasan.h.
>

Ok. I will refactor in v4.
Thanks for suggestions.

> > +{
> > +#ifdef CONFIG_KASAN_TAGS_IDENTIFY
> > + struct kasan_alloc_meta *alloc_meta;
> > + struct kmem_cache *cache;
> > + struct page *page;
> > + const void *addr;
> > + void *object;
> > + u8 tag;
> > + int i;
> > +
> > + tag = get_tag(info->access_addr);
> > + addr = kasan_reset_tag(info->access_addr);
> > + page = kasan_addr_to_page(addr);
> > + if (page && PageSlab(page)) {
> > + cache = page->slab_cache;
> > + object = nearest_obj(cache, page, (void *)addr);
> > + alloc_meta = kasan_get_alloc_meta(cache, object);
> > +
> > + if (alloc_meta) {
> > + for (i = 0; i < KASAN_NR_FREE_STACKS; i++)
> > {
> > + if (alloc_meta->free_pointer_tag[i]
> > == tag)
> > + return "use-after-free";
> > + }
> > + }
> > + return "out-of-bounds";
> > + }
> > +#endif
> > +
> > + /*
> > + * If access_size is a negative number, then it has reason
> > to be
> > + * defined as out-of-bounds bug type.
> > + *
> > + * Casting negative numbers to size_t would indeed turn up
> > as
> > + * a large size_t and its value will be larger than
> > ULONG_MAX/2,
> > + * so that this can qualify as out-of-bounds.
> > + */
> > + if (info->access_addr + info->access_size < info-
> > >access_addr)
> > + return "out-of-bounds";
> > +
> > + return "invalid-access";
> > +}