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

[Leon Romanovsky <leon@kernel.org>]

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
> > > 
> > >