Re: [PATCH v2 06/11] tpm: Add full HMAC and encrypt/decrypt session handling code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 03, 2023 at 02:06:48PM +0800, Yujie Liu wrote:
> Hi James,
> 
> On Wed, Jan 25, 2023 at 07:59:09AM -0500, James Bottomley wrote:
> > On Wed, 2023-01-25 at 07:11 +0800, kernel test robot wrote:
> > > Hi James,
> > > 
> > > I love your patch! Perhaps something to improve:
> > > 
> > > [auto build test WARNING on char-misc/char-misc-testing]
> > > [also build test WARNING on char-misc/char-misc-next char-misc/char-
> > > misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next-
> > > 20230124]
> > > [If your patch is applied to the wrong git tree, kindly drop us a
> > > note.
> > > And when submitting patch, we suggest to use '--base' as documented
> > > in
> > > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > > 
> > > url:   
> > > https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> > > patch link:   
> > > https://lore.kernel.org/r/20230124175516.5984-7-James.Bottomley%40HansenPartnership.com
> > > patch subject: [PATCH v2 06/11] tpm: Add full HMAC and
> > > encrypt/decrypt session handling code
> > > config: arc-allyesconfig
> > > (https://download.01.org/0day-ci/archive/20230125/202301250706.deGvd0
> > > yq-lkp@xxxxxxxxx/config)
> > > compiler: arceb-elf-gcc (GCC) 12.1.0
> > > reproduce (this is a W=1 build):
> > >         wget
> > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > >  -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         #
> > > https://github.com/intel-lab-lkp/linux/commit/dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > >         git remote add linux-review
> > > https://github.com/intel-lab-lkp/linux
> > >         git fetch --no-tags linux-review James-Bottomley/tpm-move-
> > > buffer-handling-from-static-inlines-to-real-functions/20230125-020146
> > >         git checkout dc0fc74718b4a786aba4a954233e8ab3afdcc03c
> > >         # save the config file
> > >         mkdir build_dir && cp config build_dir/.config
> > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > make.cross W=1 O=build_dir ARCH=arc olddefconfig
> > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> > > make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/char/tpm/
> > > 
> > > If you fix the issue, kindly add following tag where applicable
> > > > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > 
> > > All warnings (new ones prefixed by >>):
> > > 
> > >    drivers/char/tpm/tpm2-sessions.c:1184:5: warning: no previous
> > > prototype for 'tpm2_create_null_primary' [-Wmissing-prototypes]
> > >     1184 | int tpm2_create_null_primary(struct tpm_chip *chip) {
> > >          |     ^~~~~~~~~~~~~~~~~~~~~~~~
> > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > 'tpm_buf_check_hmac_response':
> > > > > drivers/char/tpm/tpm2-sessions.c:831:1: warning: the frame size
> > > > > of 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > >      831 | }
> > >          | ^
> > >    drivers/char/tpm/tpm2-sessions.c: In function
> > > 'tpm_buf_fill_hmac_session':
> > >    drivers/char/tpm/tpm2-sessions.c:579:1: warning: the frame size of
> > > 1132 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> > >      579 | }
> > >          | ^
> > 
> > Is this a test problem?  I can't see why the code would only blow the
> > stack on the arc architecture and not on any other ... does it have
> > something funny with on stack crypto structures?
> 
> This warning is controlled by the value of CONFIG_FRAME_WARN.
> 
> For "make ARCH=arc allyesconfig", the default value is 1024, so this
> frame warning shows up during the build.
> 
> For other arch such as "make ARCH=x86_64 allyesconfig", the default
> value would be 2048 and won't have this warning.
> 
> Not sure if this is a real problem that need to be fixed, here just
> providing above information for your reference.
> 
> --
> Best Regards,
> Yujie

*Must* be fixed given that it is how the default value is set now. This
is wrong place to reconsider.

And we do not want to add functions that bloat the stack this way.

Shash just needs to be allocated from heap instead of stack.

BR, Jarkko



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux