From: Kuan-Ying Lee <kylee0686026@gmail.com>
To: Marco Elver <elver@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Andrey Ryabinin <ryabinin.a.a@gmail.com>,
Alexander Potapenko <glider@google.com>,
Andrey Konovalov <andreyknvl@gmail.com>,
Dmitry Vyukov <dvyukov@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
kasan-dev <kasan-dev@googlegroups.com>,
LKML <linux-kernel@vger.kernel.org>,
Linux Memory Management List <linux-mm@kvack.org>
Subject: Re: [PATCH v2 2/3] kasan: integrate the common part of two KASAN tag-based modes
Date: Sat, 19 Jun 2021 14:39:42 +0800 [thread overview]
Message-ID: <20210619063942.GA67@DESKTOP-PJLD54P.localdomain> (raw)
In-Reply-To: <CANpmjNOf8i6HPxFb3gjTrUWMh_6c4zdsh29izrSrHDi9ud4+gw@mail.gmail.com>
On Mon, Jun 14, 2021 at 10:48:27AM +0200, Marco Elver wrote:
> On Sat, 12 Jun 2021 at 17:51, Kuan-Ying Lee <kylee0686026@gmail.com> wrote:
> [...]
> > > > diff --git a/mm/kasan/report_tags.h b/mm/kasan/report_tags.h
> > > > new file mode 100644
> > > > index 000000000000..4f740d4d99ee
> > > > --- /dev/null
> > > > +++ b/mm/kasan/report_tags.h
> > > > @@ -0,0 +1,56 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +#ifndef __MM_KASAN_REPORT_TAGS_H
> > > > +#define __MM_KASAN_REPORT_TAGS_H
> > > > +
> > > > +#include "kasan.h"
> > > > +#include "../slab.h"
> > > > +
> > > > +#ifdef CONFIG_KASAN_TAGS_IDENTIFY
> > > > +const char *kasan_get_bug_type(struct kasan_access_info *info)
> > > > +{
> > > [...]
> > > > + /*
> > > > + * 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";
> > >
> > > This seems to change behaviour for SW_TAGS because it was there even
> > > if !CONFIG_KASAN_TAGS_IDENTIFY. Does it still work as before?
> > >
> >
> > You are right. It will change the behavior.
> > However, I think that if !CONFIG_KASAN_TAG_IDENTIFY, it should be reported
> > "invalid-access".
>
> There's no reason that if !CONFIG_KASAN_TAG_IDENTIFY it should be
> reported as "invalid-acces" if we can do better without the additional
> state that the config option introduces.
>
> It's trivial to give a slightly better report without additional
> state, see the comment explaining why it's reasonable to infer
> out-of-bounds here.
>
> > Or is it better to keep it in both conditions?
>
> We want to make this patch a non-functional change.
>
Got it.
> [...]
> > > > diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c
> > > > new file mode 100644
> > > > index 000000000000..9c33c0ebe1d1
> > > > --- /dev/null
> > > > +++ b/mm/kasan/tags.c
> > > > @@ -0,0 +1,58 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * This file contains common tag-based KASAN code.
> > > > + *
> > > > + * Author: Kuan-Ying Lee <kylee0686026@gmail.com>
> > >
> > > We appreciate your work on this, but this is misleading. Because you
> > > merely copied/moved the code, have a look what sw_tags.c says -- that
> > > should either be preserved, or we add nothing here.
> > >
> > > I prefer to add nothing or the bare minimum (e.g. if the company
> > > requires a Copyright line) for non-substantial additions because this
> > > stuff becomes out-of-date fast and just isn't useful at all. 'git log'
> > > is the source of truth.
> >
> > This was my first time to upload a new file.
> > Thanks for the suggestions. :)
> > I will remove this author tag and wait for Greg's process advice.
> >
> > >
> > > Cc'ing Greg for process advice. For moved code, does it have to
> > > preserve the original Copyright line if there was one?
>
> Greg responded, see his emails. Please preserve the original header
> from the file the code was moved from (hw_tags.c/sw_tags.c).
Ok. I will do it in v3.
Thanks.
>
> Thanks,
> -- Marco
next prev parent reply other threads:[~2021-06-19 6:39 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-12 4:51 [PATCH v2 0/3] kasan: add memory corruption identification for hw tag-based kasan Kuan-Ying Lee
2021-06-12 4:51 ` [PATCH v2 1/3] kasan: rename CONFIG_KASAN_SW_TAGS_IDENTIFY to CONFIG_KASAN_TAGS_IDENTIFY Kuan-Ying Lee
2021-06-12 4:51 ` [PATCH v2 2/3] kasan: integrate the common part of two KASAN tag-based modes Kuan-Ying Lee
2021-06-12 14:42 ` Marco Elver
2021-06-12 15:51 ` Kuan-Ying Lee
2021-06-14 8:48 ` Marco Elver
2021-06-19 6:39 ` Kuan-Ying Lee [this message]
2021-06-12 15:52 ` Greg Kroah-Hartman
2021-06-12 4:51 ` [PATCH v2 3/3] kasan: add memory corruption identification support for hardware tag-based mode Kuan-Ying Lee
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=20210619063942.GA67@DESKTOP-PJLD54P.localdomain \
--to=kylee0686026@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=andreyknvl@gmail.com \
--cc=dvyukov@google.com \
--cc=elver@google.com \
--cc=glider@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=kasan-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=ryabinin.a.a@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.