diff mbox series

[3/3] kasan: stackdepot: move filter_irq_stacks() to stackdepot.c

Message ID 20200220141916.55455-3-glider@google.com (mailing list archive)
State New, archived
Headers show
Series [1/3] stackdepot: check depot_index before accessing the stack slab | expand

Commit Message

Alexander Potapenko Feb. 20, 2020, 2:19 p.m. UTC
filter_irq_stacks() can be used by other tools (e.g. KMSAN), so it needs
to be moved to a common location.
lib/stackdepot.c seems a good place, as filter_irq_stacks() is usually
applied to the output of stack_trace_save().

This patch has been previously mailed as part of KMSAN RFC patch series.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: linux-mm@kvack.org
---
 include/linux/stackdepot.h |  2 ++
 lib/stackdepot.c           | 24 ++++++++++++++++++++++++
 mm/kasan/common.c          | 23 -----------------------
 3 files changed, 26 insertions(+), 23 deletions(-)

Comments

kernel test robot Feb. 25, 2020, 8:24 p.m. UTC | #1
Hi,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.6-rc3 next-20200225]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/glider-google-com/stackdepot-check-depot_index-before-accessing-the-stack-slab/20200222-065605
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 0c0ddd6ae47c9238c18f475bcca675ca74c9dc31
config: nds32-randconfig-a001-20200225 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=9.2.0 make.cross ARCH=nds32 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   nds32le-linux-ld: lib/stackdepot.o: in function `filter_irq_stacks':
>> stackdepot.c:(.text+0xc6): undefined reference to `__softirqentry_text_start'
>> nds32le-linux-ld: stackdepot.c:(.text+0xca): undefined reference to `__softirqentry_text_start'
>> nds32le-linux-ld: stackdepot.c:(.text+0xce): undefined reference to `__softirqentry_text_end'
   nds32le-linux-ld: stackdepot.c:(.text+0xd2): undefined reference to `__softirqentry_text_end'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Alexander Potapenko Feb. 26, 2020, 9:53 a.m. UTC | #2
Hi Andes maintainers,

On Tue, Feb 25, 2020 at 9:26 PM kbuild test robot <lkp@intel.com> wrote:
>
> Hi,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v5.6-rc3 next-20200225]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url:    https://github.com/0day-ci/linux/commits/glider-google-com/stackdepot-check-depot_index-before-accessing-the-stack-slab/20200222-065605
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 0c0ddd6ae47c9238c18f475bcca675ca74c9dc31
> config: nds32-randconfig-a001-20200225 (attached as .config)
> compiler: nds32le-linux-gcc (GCC) 9.2.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=9.2.0 make.cross ARCH=nds32
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>    nds32le-linux-ld: lib/stackdepot.o: in function `filter_irq_stacks':
> >> stackdepot.c:(.text+0xc6): undefined reference to `__softirqentry_text_start'
> >> nds32le-linux-ld: stackdepot.c:(.text+0xca): undefined reference to `__softirqentry_text_start'
> >> nds32le-linux-ld: stackdepot.c:(.text+0xce): undefined reference to `__softirqentry_text_end'
>    nds32le-linux-ld: stackdepot.c:(.text+0xd2): undefined reference to `__softirqentry_text_end'

Can you please help me with this?
I couldn't find out where this section needs to be declared, and I
don't really know whether nds32 is supposed to have softirqs or not.
Maybe filter_irq_stacks() should be no-op on nds32?

> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Greentime Hu Feb. 27, 2020, 3:16 a.m. UTC | #3
Alexander Potapenko <glider@google.com> 於 2020年2月26日 週三 下午5:53寫道:
>
> Hi Andes maintainers,
>
> On Tue, Feb 25, 2020 at 9:26 PM kbuild test robot <lkp@intel.com> wrote:
> >
> > Hi,
> >
> > I love your patch! Yet something to improve:
> >
> > [auto build test ERROR on linus/master]
> > [also build test ERROR on v5.6-rc3 next-20200225]
> > [if your patch is applied to the wrong git tree, please drop us a note to help
> > improve the system. BTW, we also suggest to use '--base' option to specify the
> > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> >
> > url:    https://github.com/0day-ci/linux/commits/glider-google-com/stackdepot-check-depot_index-before-accessing-the-stack-slab/20200222-065605
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 0c0ddd6ae47c9238c18f475bcca675ca74c9dc31
> > config: nds32-randconfig-a001-20200225 (attached as .config)
> > compiler: nds32le-linux-gcc (GCC) 9.2.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         GCC_VERSION=9.2.0 make.cross ARCH=nds32
> >
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <lkp@intel.com>
> >
> > All errors (new ones prefixed by >>):
> >
> >    nds32le-linux-ld: lib/stackdepot.o: in function `filter_irq_stacks':
> > >> stackdepot.c:(.text+0xc6): undefined reference to `__softirqentry_text_start'
> > >> nds32le-linux-ld: stackdepot.c:(.text+0xca): undefined reference to `__softirqentry_text_start'
> > >> nds32le-linux-ld: stackdepot.c:(.text+0xce): undefined reference to `__softirqentry_text_end'
> >    nds32le-linux-ld: stackdepot.c:(.text+0xd2): undefined reference to `__softirqentry_text_end'
>
> Can you please help me with this?
> I couldn't find out where this section needs to be declared, and I
> don't really know whether nds32 is supposed to have softirqs or not.
> Maybe filter_irq_stacks() should be no-op on nds32?

Hi Alexander,

I've disccussed this with Nick.
There might be 2 solutions, but we are not sure which one is better or
maybe there are another better solutions.
Since nds32 doesn't support KASAN, we might not need to use this section.

Solution 1.
  We may just add the dependency in STACKDEPOT depend on KASAN

Solution 2.
  Just add SOFTIRQENTRY_TEXT this section to nds32's linker script

What do you think? :)
Alexander Potapenko March 7, 2020, 1:37 p.m. UTC | #4
On Thu, Feb 27, 2020 at 4:16 AM Greentime Hu <green.hu@gmail.com> wrote:
>
> Alexander Potapenko <glider@google.com> 於 2020年2月26日 週三 下午5:53寫道:
> >
> > Hi Andes maintainers,
> >
> > On Tue, Feb 25, 2020 at 9:26 PM kbuild test robot <lkp@intel.com> wrote:
> > >
> > > Hi,
> > >
> > > I love your patch! Yet something to improve:
> > >
> > > [auto build test ERROR on linus/master]
> > > [also build test ERROR on v5.6-rc3 next-20200225]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help
> > > improve the system. BTW, we also suggest to use '--base' option to specify the
> > > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> > >
> > > url:    https://github.com/0day-ci/linux/commits/glider-google-com/stackdepot-check-depot_index-before-accessing-the-stack-slab/20200222-065605
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 0c0ddd6ae47c9238c18f475bcca675ca74c9dc31
> > > config: nds32-randconfig-a001-20200225 (attached as .config)
> > > compiler: nds32le-linux-gcc (GCC) 9.2.0
> > > reproduce:
> > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # save the attached .config to linux build tree
> > >         GCC_VERSION=9.2.0 make.cross ARCH=nds32
> > >
> > > If you fix the issue, kindly add following tag
> > > Reported-by: kbuild test robot <lkp@intel.com>
> > >
> > > All errors (new ones prefixed by >>):
> > >
> > >    nds32le-linux-ld: lib/stackdepot.o: in function `filter_irq_stacks':
> > > >> stackdepot.c:(.text+0xc6): undefined reference to `__softirqentry_text_start'
> > > >> nds32le-linux-ld: stackdepot.c:(.text+0xca): undefined reference to `__softirqentry_text_start'
> > > >> nds32le-linux-ld: stackdepot.c:(.text+0xce): undefined reference to `__softirqentry_text_end'
> > >    nds32le-linux-ld: stackdepot.c:(.text+0xd2): undefined reference to `__softirqentry_text_end'
> >
> > Can you please help me with this?
> > I couldn't find out where this section needs to be declared, and I
> > don't really know whether nds32 is supposed to have softirqs or not.
> > Maybe filter_irq_stacks() should be no-op on nds32?
>
> Hi Alexander,
>
> I've disccussed this with Nick.
> There might be 2 solutions, but we are not sure which one is better or
> maybe there are another better solutions.
> Since nds32 doesn't support KASAN, we might not need to use this section.
>
> Solution 1.
>   We may just add the dependency in STACKDEPOT depend on KASAN
>
> Solution 2.
>   Just add SOFTIRQENTRY_TEXT this section to nds32's linker script
>
> What do you think? :)
Hi,

I think the latter solution actually makes sense.
diff mbox series

Patch

diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
index 3efa97d482cbe..24d49c732341a 100644
--- a/include/linux/stackdepot.h
+++ b/include/linux/stackdepot.h
@@ -19,4 +19,6 @@  depot_stack_handle_t stack_depot_save(unsigned long *entries,
 unsigned int stack_depot_fetch(depot_stack_handle_t handle,
 			       unsigned long **entries);
 
+unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries);
+
 #endif
diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index a2f6cb900db80..da5d1880bf343 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -20,6 +20,7 @@ 
  */
 
 #include <linux/gfp.h>
+#include <linux/interrupt.h>
 #include <linux/jhash.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
@@ -318,3 +319,26 @@  depot_stack_handle_t stack_depot_save(unsigned long *entries,
 	return retval;
 }
 EXPORT_SYMBOL_GPL(stack_depot_save);
+
+static inline int in_irqentry_text(unsigned long ptr)
+{
+	return (ptr >= (unsigned long)&__irqentry_text_start &&
+		ptr < (unsigned long)&__irqentry_text_end) ||
+		(ptr >= (unsigned long)&__softirqentry_text_start &&
+		 ptr < (unsigned long)&__softirqentry_text_end);
+}
+
+unsigned int filter_irq_stacks(unsigned long *entries,
+					     unsigned int nr_entries)
+{
+	unsigned int i;
+
+	for (i = 0; i < nr_entries; i++) {
+		if (in_irqentry_text(entries[i])) {
+			/* Include the irqentry function into the stack. */
+			return i + 1;
+		}
+	}
+	return nr_entries;
+}
+EXPORT_SYMBOL_GPL(filter_irq_stacks);
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index 6aa51723b92b9..92815ef9c80d3 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -15,7 +15,6 @@ 
  */
 
 #include <linux/export.h>
-#include <linux/interrupt.h>
 #include <linux/init.h>
 #include <linux/kasan.h>
 #include <linux/kernel.h>
@@ -42,28 +41,6 @@ 
 #include "kasan.h"
 #include "../slab.h"
 
-static inline int in_irqentry_text(unsigned long ptr)
-{
-	return (ptr >= (unsigned long)&__irqentry_text_start &&
-		ptr < (unsigned long)&__irqentry_text_end) ||
-		(ptr >= (unsigned long)&__softirqentry_text_start &&
-		 ptr < (unsigned long)&__softirqentry_text_end);
-}
-
-static inline unsigned int filter_irq_stacks(unsigned long *entries,
-					     unsigned int nr_entries)
-{
-	unsigned int i;
-
-	for (i = 0; i < nr_entries; i++) {
-		if (in_irqentry_text(entries[i])) {
-			/* Include the irqentry function into the stack. */
-			return i + 1;
-		}
-	}
-	return nr_entries;
-}
-
 static inline depot_stack_handle_t save_stack(gfp_t flags)
 {
 	unsigned long entries[KASAN_STACK_DEPTH];