2022-02-24 01:30:05

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [kbuild-all] Re: [PATCH v2 6/9] KVM: arm64: Detect and handle hypervisor stack overflows

On Wed, 23 Feb 2022 at 13:54, Marc Zyngier <[email protected]> wrote:
>
> On 2022-02-23 12:34, Philip Li wrote:
> > On Wed, Feb 23, 2022 at 09:16:59AM +0000, Marc Zyngier wrote:
> >> On Wed, 23 Feb 2022 09:05:18 +0000,
> >> kernel test robot <[email protected]> wrote:
> >> >
> >> > Hi Kalesh,
> >> >
> >> > Thank you for the patch! Perhaps something to improve:
> >> >
> >> > [auto build test WARNING on cfb92440ee71adcc2105b0890bb01ac3cddb8507]
> >> >
> >> > url: https://github.com/0day-ci/linux/commits/Kalesh-Singh/KVM-arm64-Hypervisor-stack-enhancements/20220223-010522
> >> > base: cfb92440ee71adcc2105b0890bb01ac3cddb8507
> >> > config: arm64-randconfig-r011-20220221 (https://download.01.org/0day-ci/archive/20220223/[email protected]/config)
> >> > compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
> >> > 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
> >> > # install arm64 cross compiling tool for clang build
> >> > # apt-get install binutils-aarch64-linux-gnu
> >> > # https://github.com/0day-ci/linux/commit/7fe99fd40f7c4b2973218045ca5b9c9160524db1
> >> > git remote add linux-review https://github.com/0day-ci/linux
> >> > git fetch --no-tags linux-review Kalesh-Singh/KVM-arm64-Hypervisor-stack-enhancements/20220223-010522
> >> > git checkout 7fe99fd40f7c4b2973218045ca5b9c9160524db1
> >> > # save the config file to linux build tree
> >> > mkdir build_dir
> >> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash arch/arm64/
> >> >
> >> > If you fix the issue, kindly add following tag as appropriate
> >> > Reported-by: kernel test robot <[email protected]>
> >> >
> >> > All warnings (new ones prefixed by >>):
> >> >
> >> > include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
> >> > #define NULL ((void *)0)
> >> > ^~~~~~~~~~~
> >> > arch/arm64/kvm/hyp/nvhe/switch.c:200:27: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
> >> > [ESR_ELx_EC_FP_ASIMD] = kvm_hyp_handle_fpsimd,
> >> > ^~~~~~~~~~~~~~~~~~~~~
> >> > arch/arm64/kvm/hyp/nvhe/switch.c:196:28: note: previous initialization is here
> >> > [0 ... ESR_ELx_EC_MAX] = NULL,
> >> > ^~~~
> >> > include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
> >> > #define NULL ((void *)0)
> >> > ^~~~~~~~~~~
> >>
> >> Kalesh, please ignore this nonsense. There may be things to improve,
> >> but this is *NOT* one of them.
> >>
> >> These reports are pretty useless, and just lead people to ignore real
> >> bug reports.
> >
> > Hi Kalesh, sorry there're some irrelevant issues mixed in the report,
> > kindly ignore them. And the valuable ones are the new ones that
> > prefixed by >>, as the below one in original report.
> >
> >>> arch/arm64/kvm/hyp/nvhe/switch.c:372:17: warning: no previous
> >>> prototype for function 'hyp_panic_bad_stack' [-Wmissing-prototypes]
> > void __noreturn hyp_panic_bad_stack(void)
> > ^
>
> This is only called from assembly code, so a prototype wouldn't bring
> much.
>

Should probably be marked as 'asmlinkage' then. I've suggested many
times already that this bogus diagnostic should either be disabled, or
disregard 'asmlinkage' symbols.


2022-02-24 10:58:40

by Marc Zyngier

[permalink] [raw]
Subject: Re: [kbuild-all] Re: [PATCH v2 6/9] KVM: arm64: Detect and handle hypervisor stack overflows

On 2022-02-23 12:56, Ard Biesheuvel wrote:
> On Wed, 23 Feb 2022 at 13:54, Marc Zyngier <[email protected]> wrote:
>>
>> On 2022-02-23 12:34, Philip Li wrote:
>> > On Wed, Feb 23, 2022 at 09:16:59AM +0000, Marc Zyngier wrote:
>> >> On Wed, 23 Feb 2022 09:05:18 +0000,
>> >> kernel test robot <[email protected]> wrote:
>> >> >
>> >> > Hi Kalesh,
>> >> >
>> >> > Thank you for the patch! Perhaps something to improve:
>> >> >
>> >> > [auto build test WARNING on cfb92440ee71adcc2105b0890bb01ac3cddb8507]
>> >> >
>> >> > url: https://github.com/0day-ci/linux/commits/Kalesh-Singh/KVM-arm64-Hypervisor-stack-enhancements/20220223-010522
>> >> > base: cfb92440ee71adcc2105b0890bb01ac3cddb8507
>> >> > config: arm64-randconfig-r011-20220221 (https://download.01.org/0day-ci/archive/20220223/[email protected]/config)
>> >> > compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
>> >> > 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
>> >> > # install arm64 cross compiling tool for clang build
>> >> > # apt-get install binutils-aarch64-linux-gnu
>> >> > # https://github.com/0day-ci/linux/commit/7fe99fd40f7c4b2973218045ca5b9c9160524db1
>> >> > git remote add linux-review https://github.com/0day-ci/linux
>> >> > git fetch --no-tags linux-review Kalesh-Singh/KVM-arm64-Hypervisor-stack-enhancements/20220223-010522
>> >> > git checkout 7fe99fd40f7c4b2973218045ca5b9c9160524db1
>> >> > # save the config file to linux build tree
>> >> > mkdir build_dir
>> >> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash arch/arm64/
>> >> >
>> >> > If you fix the issue, kindly add following tag as appropriate
>> >> > Reported-by: kernel test robot <[email protected]>
>> >> >
>> >> > All warnings (new ones prefixed by >>):
>> >> >
>> >> > include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
>> >> > #define NULL ((void *)0)
>> >> > ^~~~~~~~~~~
>> >> > arch/arm64/kvm/hyp/nvhe/switch.c:200:27: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
>> >> > [ESR_ELx_EC_FP_ASIMD] = kvm_hyp_handle_fpsimd,
>> >> > ^~~~~~~~~~~~~~~~~~~~~
>> >> > arch/arm64/kvm/hyp/nvhe/switch.c:196:28: note: previous initialization is here
>> >> > [0 ... ESR_ELx_EC_MAX] = NULL,
>> >> > ^~~~
>> >> > include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
>> >> > #define NULL ((void *)0)
>> >> > ^~~~~~~~~~~
>> >>
>> >> Kalesh, please ignore this nonsense. There may be things to improve,
>> >> but this is *NOT* one of them.
>> >>
>> >> These reports are pretty useless, and just lead people to ignore real
>> >> bug reports.
>> >
>> > Hi Kalesh, sorry there're some irrelevant issues mixed in the report,
>> > kindly ignore them. And the valuable ones are the new ones that
>> > prefixed by >>, as the below one in original report.
>> >
>> >>> arch/arm64/kvm/hyp/nvhe/switch.c:372:17: warning: no previous
>> >>> prototype for function 'hyp_panic_bad_stack' [-Wmissing-prototypes]
>> > void __noreturn hyp_panic_bad_stack(void)
>> > ^
>>
>> This is only called from assembly code, so a prototype wouldn't bring
>> much.
>>
>
> Should probably be marked as 'asmlinkage' then. I've suggested many
> times already that this bogus diagnostic should either be disabled, or
> disregard 'asmlinkage' symbols.

Yes, asmlinkage is definitely missing.

But it is pretty obvious that the robot people aren't interested in
fixing this particular issue, given how long we have been suggesting
this...

M.
--
Jazz is not dead. It just smells funny...

2022-02-25 08:13:03

by Chen, Rong A

[permalink] [raw]
Subject: Re: [kbuild-all] Re: [PATCH v2 6/9] KVM: arm64: Detect and handle hypervisor stack overflows



On 2/24/2022 6:39 PM, Marc Zyngier wrote:
> On 2022-02-23 12:56, Ard Biesheuvel wrote:
>> On Wed, 23 Feb 2022 at 13:54, Marc Zyngier <[email protected]> wrote:
>>>
>>> On 2022-02-23 12:34, Philip Li wrote:
>>> > On Wed, Feb 23, 2022 at 09:16:59AM +0000, Marc Zyngier wrote:
>>> >> On Wed, 23 Feb 2022 09:05:18 +0000,
>>> >> kernel test robot <[email protected]> wrote:
>>> >> >
>>> >> > Hi Kalesh,
>>> >> >
>>> >> > Thank you for the patch! Perhaps something to improve:
>>> >> >
>>> >> > [auto build test WARNING on
>>> cfb92440ee71adcc2105b0890bb01ac3cddb8507]
>>> >> >
>>> >> > url:
>>> https://github.com/0day-ci/linux/commits/Kalesh-Singh/KVM-arm64-Hypervisor-stack-enhancements/20220223-010522
>>>
>>> >> > base:   cfb92440ee71adcc2105b0890bb01ac3cddb8507
>>> >> > config: arm64-randconfig-r011-20220221
>>> (https://download.01.org/0day-ci/archive/20220223/[email protected]/config)
>>>
>>> >> > compiler: clang version 15.0.0
>>> (https://github.com/llvm/llvm-project
>>> d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
>>> >> > 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
>>> >> >         # install arm64 cross compiling tool for clang build
>>> >> >         # apt-get install binutils-aarch64-linux-gnu
>>> >> >         #
>>> https://github.com/0day-ci/linux/commit/7fe99fd40f7c4b2973218045ca5b9c9160524db1
>>>
>>> >> >         git remote add linux-review
>>> https://github.com/0day-ci/linux
>>> >> >         git fetch --no-tags linux-review
>>> Kalesh-Singh/KVM-arm64-Hypervisor-stack-enhancements/20220223-010522
>>> >> >         git checkout 7fe99fd40f7c4b2973218045ca5b9c9160524db1
>>> >> >         # save the config file to linux build tree
>>> >> >         mkdir build_dir
>>> >> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang
>>> make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash arch/arm64/
>>> >> >
>>> >> > If you fix the issue, kindly add following tag as appropriate
>>> >> > Reported-by: kernel test robot <[email protected]>
>>> >> >
>>> >> > All warnings (new ones prefixed by >>):
>>> >> >
>>> >> >    include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
>>> >> >    #define NULL ((void *)0)
>>> >> >                 ^~~~~~~~~~~
>>> >> >    arch/arm64/kvm/hyp/nvhe/switch.c:200:27: warning: initializer
>>> overrides prior initialization of this subobject
>>> [-Winitializer-overrides]
>>> >> >            [ESR_ELx_EC_FP_ASIMD]           = kvm_hyp_handle_fpsimd,
>>> >> >                                              ^~~~~~~~~~~~~~~~~~~~~
>>> >> >    arch/arm64/kvm/hyp/nvhe/switch.c:196:28: note: previous
>>> initialization is here
>>> >> >            [0 ... ESR_ELx_EC_MAX]          = NULL,
>>> >> >                                              ^~~~
>>> >> >    include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
>>> >> >    #define NULL ((void *)0)
>>> >> >                 ^~~~~~~~~~~
>>> >>
>>> >> Kalesh, please ignore this nonsense. There may be things to improve,
>>> >> but this is *NOT* one of them.
>>> >>
>>> >> These reports are pretty useless, and just lead people to ignore real
>>> >> bug reports.
>>> >
>>> > Hi Kalesh, sorry there're some irrelevant issues mixed in the report,
>>> > kindly ignore them. And the valuable ones are the new ones that
>>> > prefixed by >>, as the below one in original report.
>>> >
>>> >>> arch/arm64/kvm/hyp/nvhe/switch.c:372:17: warning: no previous
>>> >>> prototype for function 'hyp_panic_bad_stack' [-Wmissing-prototypes]
>>> >    void __noreturn hyp_panic_bad_stack(void)
>>> >                    ^
>>>
>>> This is only called from assembly code, so a prototype wouldn't bring
>>> much.
>>>
>>
>> Should probably be marked as 'asmlinkage' then. I've suggested many
>> times already that this bogus diagnostic should either be disabled, or
>> disregard 'asmlinkage' symbols.
>
> Yes, asmlinkage is definitely missing.
>
> But it is pretty obvious that the robot people aren't interested in
> fixing this particular issue, given how long we have been suggesting
> this...
>
>         M.

Hi Marc, Ard,

We have ignored the warning related to asmlinkage according to the below
advice:

https://lore.kernel.org/lkml/CAMj1kXHrRYagSVniSetHdG15rkQS+fm4zVOtN=Zda3W0QaEoJA@mail.gmail.com/

do you want the bot ignore such warning if asmlinkage not specified?

Best Regards,
Rong Chen

2022-02-25 19:30:45

by Marc Zyngier

[permalink] [raw]
Subject: Re: [kbuild-all] Re: [PATCH v2 6/9] KVM: arm64: Detect and handle hypervisor stack overflows

On Fri, 25 Feb 2022 02:12:32 +0000,
"Chen, Rong A" <[email protected]> wrote:
>
> Hi Marc, Ard,
>
> We have ignored the warning related to asmlinkage according to the
> below advice:
>
> https://lore.kernel.org/lkml/CAMj1kXHrRYagSVniSetHdG15rkQS+fm4zVOtN=Zda3W0QaEoJA@mail.gmail.com/

Ah, I didn't realise it had been updated. Apologies for the shouting!

> do you want the bot ignore such warning if asmlinkage not specified?

No, there is some value in warning about this particular case.

Thanks,

M.

--
Without deviation from the norm, progress is not possible.

2022-02-26 02:28:42

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [kbuild-all] Re: [PATCH v2 6/9] KVM: arm64: Detect and handle hypervisor stack overflows

On Fri, 25 Feb 2022 at 03:12, Chen, Rong A <[email protected]> wrote:
>
>
>

> Hi Marc, Ard,
>
> We have ignored the warning related to asmlinkage according to the below
> advice:
>
> https://lore.kernel.org/lkml/CAMj1kXHrRYagSVniSetHdG15rkQS+fm4zVOtN=Zda3W0QaEoJA@mail.gmail.com/
>

Excellent! Thanks for implementing this - I wasn't aware that you
adopted this suggestion.

> do you want the bot ignore such warning if asmlinkage not specified?
>

Even though I think this warning has little value, I think asmlinkage
is sufficient for us to avoid it for symbols that are exported for use
by assembler code.

So I don't think this additional change is needed.

--
Ard.