[PATCH] infiniband/hw/ocrdma: increase frame warning limit in verifier when using KASAN or KCSAN

[PATCH] infiniband/hw/ocrdma: increase frame warning limit in verifier when using KASAN or KCSAN

[flyingpenghao]

From: Peng Hao <flyingpeng@tencent.com>

When building kernel with clang, which will typically
have sanitizers enabled, there is a warning about a large stack frame.

drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame size (20664) exceeds limit (8192) in 'ocrdma_dbgfs_ops_read' [-Werror,-Wframe-larger-than]
static ssize_t ocrdma_dbgfs_ops_read(struct file *filp, char __user *buffer,
               ^

Increase the frame size by 20% to set.

Signed-off-by: Peng Hao <flyingpeng@tencent.com>
---
 drivers/infiniband/hw/ocrdma/Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/infiniband/hw/ocrdma/Makefile b/drivers/infiniband/hw/ocrdma/Makefile
index 14fba95021d8..a1e9fcc04751 100644
--- a/drivers/infiniband/hw/ocrdma/Makefile
+++ b/drivers/infiniband/hw/ocrdma/Makefile
@@ -3,4 +3,10 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/emulex/benet
 
 obj-$(CONFIG_INFINIBAND_OCRDMA)	+= ocrdma.o
 
+ifneq ($(CONFIG_FRAME_WARN),0)
+ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y)
+CFLAGS_ocrdma_stats.o = -Wframe-larger-than=22664
+endif
+endif
+
 ocrdma-y :=	ocrdma_main.o ocrdma_verbs.o ocrdma_hw.o ocrdma_ah.o ocrdma_stats.o
-- 
2.27.0

Re: [PATCH] infiniband/hw/ocrdma: increase frame warning limit in verifier when using KASAN or KCSAN

[Leon Romanovsky]

On Tue, Jul 09, 2024 at 06:52:42PM +0800, flyingpenghao@gmail.com wrote:
> From: Peng Hao <flyingpeng@tencent.com>
> 
> When building kernel with clang, which will typically
> have sanitizers enabled, there is a warning about a large stack frame.
> 
> drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame size (20664) exceeds limit (8192) in 'ocrdma_dbgfs_ops_read' [-Werror,-Wframe-larger-than]
> static ssize_t ocrdma_dbgfs_ops_read(struct file *filp, char __user *buffer,
>                ^

Please fix it, not hide it.

Thanks

> 
> Increase the frame size by 20% to set.
> 
> Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> ---
>  drivers/infiniband/hw/ocrdma/Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/Makefile b/drivers/infiniband/hw/ocrdma/Makefile
> index 14fba95021d8..a1e9fcc04751 100644
> --- a/drivers/infiniband/hw/ocrdma/Makefile
> +++ b/drivers/infiniband/hw/ocrdma/Makefile
> @@ -3,4 +3,10 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/emulex/benet
>  
>  obj-$(CONFIG_INFINIBAND_OCRDMA)	+= ocrdma.o
>  
> +ifneq ($(CONFIG_FRAME_WARN),0)
> +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y)
> +CFLAGS_ocrdma_stats.o = -Wframe-larger-than=22664
> +endif
> +endif
> +
>  ocrdma-y :=	ocrdma_main.o ocrdma_verbs.o ocrdma_hw.o ocrdma_ah.o ocrdma_stats.o
> -- 
> 2.27.0
> 
> 

Re: [PATCH] infiniband/hw/ocrdma: increase frame warning limit in verifier when using KASAN or KCSAN

[Nathan Chancellor]

On Tue, Jul 09, 2024 at 03:27:37PM +0300, Leon Romanovsky wrote:
> On Tue, Jul 09, 2024 at 06:52:42PM +0800, flyingpenghao@gmail.com wrote:
> > From: Peng Hao <flyingpeng@tencent.com>
> > 
> > When building kernel with clang, which will typically
> > have sanitizers enabled, there is a warning about a large stack frame.
> > 
> > drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame size (20664) exceeds limit (8192) in 'ocrdma_dbgfs_ops_read' [-Werror,-Wframe-larger-than]
> > static ssize_t ocrdma_dbgfs_ops_read(struct file *filp, char __user *buffer,
> >                ^
> 
> Please fix it, not hide it.

Agreed, this is far from an acceptable solution. No details were
provided around compiler, architecture, or configuration, so I can only
speculate what is happening here. From reading the code, I suspect that
ocrdma_add_stat() is getting inlined into all of its callsites but the
stack slot for buff[128] is not getting reused, which may be related to
a missing lifetime marker like [1] or sanitizer instrumentation.  I am
guessing that marking ocrdma_dbgfs_ops_read() as noinline_for_stack
would resolve this.

static noinline_for_stack int ocrdma_add_stat(char *start, char *pcur,

If this is not tolerable for all configurations, it could be made more
pointed with something like

static
#if defined(CONFIG_CC_IS_CLANG) && (defined(CONFIG_KASAN) || defined(CONFIG_KCSAN))
noinline_for_stack
#endif
int ocrdma_add_stat(char *start, char *pcur,

but I am aware that is quite ugly.

[1]: https://github.com/llvm/llvm-project/issues/38157#issuecomment-1756321571

Cheers,
Nathan

> > Increase the frame size by 20% to set.
> > 
> > Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> > ---
> >  drivers/infiniband/hw/ocrdma/Makefile | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/infiniband/hw/ocrdma/Makefile b/drivers/infiniband/hw/ocrdma/Makefile
> > index 14fba95021d8..a1e9fcc04751 100644
> > --- a/drivers/infiniband/hw/ocrdma/Makefile
> > +++ b/drivers/infiniband/hw/ocrdma/Makefile
> > @@ -3,4 +3,10 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/emulex/benet
> >  
> >  obj-$(CONFIG_INFINIBAND_OCRDMA)	+= ocrdma.o
> >  
> > +ifneq ($(CONFIG_FRAME_WARN),0)
> > +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y)
> > +CFLAGS_ocrdma_stats.o = -Wframe-larger-than=22664
> > +endif
> > +endif
> > +
> >  ocrdma-y :=	ocrdma_main.o ocrdma_verbs.o ocrdma_hw.o ocrdma_ah.o ocrdma_stats.o
> > -- 
> > 2.27.0
> > 
> > 

Re: [PATCH] infiniband/hw/ocrdma: increase frame warning limit in verifier when using KASAN or KCSAN

[Leon Romanovsky]

On Tue, Jul 09, 2024 at 02:26:08PM -0700, Nathan Chancellor wrote:
> On Tue, Jul 09, 2024 at 03:27:37PM +0300, Leon Romanovsky wrote:
> > On Tue, Jul 09, 2024 at 06:52:42PM +0800, flyingpenghao@gmail.com wrote:
> > > From: Peng Hao <flyingpeng@tencent.com>
> > > 
> > > When building kernel with clang, which will typically
> > > have sanitizers enabled, there is a warning about a large stack frame.
> > > 
> > > drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame size (20664) exceeds limit (8192) in 'ocrdma_dbgfs_ops_read' [-Werror,-Wframe-larger-than]
> > > static ssize_t ocrdma_dbgfs_ops_read(struct file *filp, char __user *buffer,
> > >                ^
> > 
> > Please fix it, not hide it.
> 
> Agreed, this is far from an acceptable solution. No details were
> provided around compiler, architecture, or configuration, so I can only
> speculate what is happening here. From reading the code, I suspect that
> ocrdma_add_stat() is getting inlined into all of its callsites but the
> stack slot for buff[128] is not getting reused, which may be related to
> a missing lifetime marker like [1] or sanitizer instrumentation.  I am
> guessing that marking ocrdma_dbgfs_ops_read() as noinline_for_stack
> would resolve this.
> 
> static noinline_for_stack int ocrdma_add_stat(char *start, char *pcur,

This is a good solution, but first let's see if the author can provide
more details. 

> 
> If this is not tolerable for all configurations, it could be made more
> pointed with something like
> 
> static
> #if defined(CONFIG_CC_IS_CLANG) && (defined(CONFIG_KASAN) || defined(CONFIG_KCSAN))
> noinline_for_stack
> #endif
> int ocrdma_add_stat(char *start, char *pcur,
> 
> but I am aware that is quite ugly.
> 
> [1]: https://github.com/llvm/llvm-project/issues/38157#issuecomment-1756321571
> 
> Cheers,
> Nathan
> 
> > > Increase the frame size by 20% to set.
> > > 
> > > Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> > > ---
> > >  drivers/infiniband/hw/ocrdma/Makefile | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/infiniband/hw/ocrdma/Makefile b/drivers/infiniband/hw/ocrdma/Makefile
> > > index 14fba95021d8..a1e9fcc04751 100644
> > > --- a/drivers/infiniband/hw/ocrdma/Makefile
> > > +++ b/drivers/infiniband/hw/ocrdma/Makefile
> > > @@ -3,4 +3,10 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/emulex/benet
> > >  
> > >  obj-$(CONFIG_INFINIBAND_OCRDMA)	+= ocrdma.o
> > >  
> > > +ifneq ($(CONFIG_FRAME_WARN),0)
> > > +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y)
> > > +CFLAGS_ocrdma_stats.o = -Wframe-larger-than=22664
> > > +endif
> > > +endif
> > > +
> > >  ocrdma-y :=	ocrdma_main.o ocrdma_verbs.o ocrdma_hw.o ocrdma_ah.o ocrdma_stats.o
> > > -- 
> > > 2.27.0
> > > 
> > > 

Re: [PATCH] infiniband/hw/ocrdma: increase frame warning limit in verifier when using KASAN or KCSAN

[Hao Peng]

On Wed, Jul 10, 2024 at 2:02 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Tue, Jul 09, 2024 at 02:26:08PM -0700, Nathan Chancellor wrote:
> > On Tue, Jul 09, 2024 at 03:27:37PM +0300, Leon Romanovsky wrote:
> > > On Tue, Jul 09, 2024 at 06:52:42PM +0800, flyingpenghao@gmail.com wrote:
> > > > From: Peng Hao <flyingpeng@tencent.com>
> > > >
> > > > When building kernel with clang, which will typically
> > > > have sanitizers enabled, there is a warning about a large stack frame.
> > > >
> > > > drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame size (20664) exceeds limit (8192) in 'ocrdma_dbgfs_ops_read' [-Werror,-Wframe-larger-than]
> > > > static ssize_t ocrdma_dbgfs_ops_read(struct file *filp, char __user *buffer,
> > > >                ^
> > >
> > > Please fix it, not hide it.
> >
> > Agreed, this is far from an acceptable solution. No details were
> > provided around compiler, architecture, or configuration, so I can only
> > speculate what is happening here. From reading the code, I suspect that
> > ocrdma_add_stat() is getting inlined into all of its callsites but the
> > stack slot for buff[128] is not getting reused, which may be related to
> > a missing lifetime marker like [1] or sanitizer instrumentation.  I am
> > guessing that marking ocrdma_dbgfs_ops_read() as noinline_for_stack
> > would resolve this.
> >
KASAN is generally enabled for testing environments;
noinline_for_stack is used to solve the problem of needing to
 track deeper call chains. I also tried to use noinline_for_stack and
issued a new patch, but I think the v1 method is better.
My environment is:
x86_64, clang version 15.0.7
CONFIG_FRAME_WARN=8192
> > static noinline_for_stack int ocrdma_add_stat(char *start, char *pcur,
>
> This is a good solution, but first let's see if the author can provide
> more details.
>
> >
> > If this is not tolerable for all configurations, it could be made more
> > pointed with something like
> >
> > static
> > #if defined(CONFIG_CC_IS_CLANG) && (defined(CONFIG_KASAN) || defined(CONFIG_KCSAN))
> > noinline_for_stack
> > #endif
> > int ocrdma_add_stat(char *start, char *pcur,
> >
> > but I am aware that is quite ugly.
> >
> > [1]: https://github.com/llvm/llvm-project/issues/38157#issuecomment-1756321571
> >
> > Cheers,
> > Nathan
> >
> > > > Increase the frame size by 20% to set.
> > > >
> > > > Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> > > > ---
> > > >  drivers/infiniband/hw/ocrdma/Makefile | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/drivers/infiniband/hw/ocrdma/Makefile b/drivers/infiniband/hw/ocrdma/Makefile
> > > > index 14fba95021d8..a1e9fcc04751 100644
> > > > --- a/drivers/infiniband/hw/ocrdma/Makefile
> > > > +++ b/drivers/infiniband/hw/ocrdma/Makefile
> > > > @@ -3,4 +3,10 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/emulex/benet
> > > >
> > > >  obj-$(CONFIG_INFINIBAND_OCRDMA)  += ocrdma.o
> > > >
> > > > +ifneq ($(CONFIG_FRAME_WARN),0)
> > > > +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y)
> > > > +CFLAGS_ocrdma_stats.o = -Wframe-larger-than=22664
> > > > +endif
> > > > +endif
> > > > +
> > > >  ocrdma-y :=      ocrdma_main.o ocrdma_verbs.o ocrdma_hw.o ocrdma_ah.o ocrdma_stats.o
> > > > --
> > > > 2.27.0
> > > >
> > > >

Re: [PATCH] infiniband/hw/ocrdma: increase frame warning limit in verifier when using KASAN or KCSAN

[Nathan Chancellor]

On Wed, Jul 10, 2024 at 09:46:27PM +0800, Hao Peng wrote:
> On Wed, Jul 10, 2024 at 2:02 PM Leon Romanovsky <leon@kernel.org> wrote:
> >
> > On Tue, Jul 09, 2024 at 02:26:08PM -0700, Nathan Chancellor wrote:
> > > On Tue, Jul 09, 2024 at 03:27:37PM +0300, Leon Romanovsky wrote:
> > > > On Tue, Jul 09, 2024 at 06:52:42PM +0800, flyingpenghao@gmail.com wrote:
> > > > > From: Peng Hao <flyingpeng@tencent.com>
> > > > >
> > > > > When building kernel with clang, which will typically
> > > > > have sanitizers enabled, there is a warning about a large stack frame.
> > > > >
> > > > > drivers/infiniband/hw/ocrdma/ocrdma_stats.c:686:16: error: stack frame size (20664) exceeds limit (8192) in 'ocrdma_dbgfs_ops_read' [-Werror,-Wframe-larger-than]
> > > > > static ssize_t ocrdma_dbgfs_ops_read(struct file *filp, char __user *buffer,
> > > > >                ^
> > > >
> > > > Please fix it, not hide it.
> > >
> > > Agreed, this is far from an acceptable solution. No details were
> > > provided around compiler, architecture, or configuration, so I can only
> > > speculate what is happening here. From reading the code, I suspect that
> > > ocrdma_add_stat() is getting inlined into all of its callsites but the
> > > stack slot for buff[128] is not getting reused, which may be related to
> > > a missing lifetime marker like [1] or sanitizer instrumentation.  I am
> > > guessing that marking ocrdma_dbgfs_ops_read() as noinline_for_stack

This should have been ocrdma_add_stat()...

> > > would resolve this.
> > >
> KASAN is generally enabled for testing environments;
> noinline_for_stack is used to solve the problem of needing to
>  track deeper call chains. I also tried to use noinline_for_stack and
> issued a new patch, but I think the v1 method is better.

As far as I understand it, this stack usage would be a problem at runtime if
running without VMAP_STACK, so I am going to disagree. v2 gets more at the
heart of the problem (although I wonder if that just moves the high
stack usage to those other functions under the limit).

> My environment is:
> x86_64, clang version 15.0.7
> CONFIG_FRAME_WARN=8192

Is CONFIG_KASAN_STACK enabled? I realized that could be another reason
you see this warning, as that option is known to have high stack usage
with clang.

> > > static noinline_for_stack int ocrdma_add_stat(char *start, char *pcur,
> >
> > This is a good solution, but first let's see if the author can provide
> > more details.
> >
> > >
> > > If this is not tolerable for all configurations, it could be made more
> > > pointed with something like
> > >
> > > static
> > > #if defined(CONFIG_CC_IS_CLANG) && (defined(CONFIG_KASAN) || defined(CONFIG_KCSAN))
> > > noinline_for_stack
> > > #endif
> > > int ocrdma_add_stat(char *start, char *pcur,
> > >
> > > but I am aware that is quite ugly.
> > >
> > > [1]: https://github.com/llvm/llvm-project/issues/38157#issuecomment-1756321571
> > >
> > > Cheers,
> > > Nathan
> > >
> > > > > Increase the frame size by 20% to set.
> > > > >
> > > > > Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> > > > > ---
> > > > >  drivers/infiniband/hw/ocrdma/Makefile | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/drivers/infiniband/hw/ocrdma/Makefile b/drivers/infiniband/hw/ocrdma/Makefile
> > > > > index 14fba95021d8..a1e9fcc04751 100644
> > > > > --- a/drivers/infiniband/hw/ocrdma/Makefile
> > > > > +++ b/drivers/infiniband/hw/ocrdma/Makefile
> > > > > @@ -3,4 +3,10 @@ ccflags-y := -I $(srctree)/drivers/net/ethernet/emulex/benet
> > > > >
> > > > >  obj-$(CONFIG_INFINIBAND_OCRDMA)  += ocrdma.o
> > > > >
> > > > > +ifneq ($(CONFIG_FRAME_WARN),0)
> > > > > +ifeq ($(filter y,$(CONFIG_KASAN)$(CONFIG_KCSAN)),y)
> > > > > +CFLAGS_ocrdma_stats.o = -Wframe-larger-than=22664
> > > > > +endif
> > > > > +endif
> > > > > +
> > > > >  ocrdma-y :=      ocrdma_main.o ocrdma_verbs.o ocrdma_hw.o ocrdma_ah.o ocrdma_stats.o
> > > > > --
> > > > > 2.27.0
> > > > >
> > > > >