Re: [PATCH] acpi: acpica: add acpi status check prior walking through namespace
On Wed, Dec 13, 2017 at 05:48:49PM +0100, Rafael J. Wysocki wrote: > On Wed, Dec 13, 2017 at 3:52 PM, Vadim Lomovtsev > wrote: > > On Tue, Dec 12, 2017 at 09:52:21PM +, Moore, Robert wrote: > >> Another way to look at this is that the kernel should not be calling ACPI > >> interfaces if ACPI is disabled. > > > > Yes, I agree. So in this case the ltp_acpi test case has to be updated with > > such checking > > before calling ACPI interfaces. However, it seems that such calls was put > > there intentionally, > > without ACPI state check, as part of kernel testing strategy. > > Not really. Ok, see your point. Thank you for your time. WBR, Vadim > > Thanks, > Rafael
Re: [PATCH] acpi: acpica: add acpi status check prior walking through namespace
On Wed, Dec 13, 2017 at 3:52 PM, Vadim Lomovtsev wrote: > On Tue, Dec 12, 2017 at 09:52:21PM +, Moore, Robert wrote: >> Another way to look at this is that the kernel should not be calling ACPI >> interfaces if ACPI is disabled. > > Yes, I agree. So in this case the ltp_acpi test case has to be updated with > such checking > before calling ACPI interfaces. However, it seems that such calls was put > there intentionally, > without ACPI state check, as part of kernel testing strategy. Not really. Thanks, Rafael
Re: [PATCH] acpi: acpica: add acpi status check prior walking through namespace
On Tue, Dec 12, 2017 at 09:52:21PM +, Moore, Robert wrote:
> Another way to look at this is that the kernel should not be calling ACPI
> interfaces if ACPI is disabled.
Yes, I agree. So in this case the ltp_acpi test case has to be updated with
such checking
before calling ACPI interfaces. However, it seems that such calls was put there
intentionally,
without ACPI state check, as part of kernel testing strategy.
WBR,
Vadim
>
> > -Original Message-
> > From: Vadim Lomovtsev [mailto:vadim.lomovt...@caviumnetworks.com]
> > Sent: Tuesday, December 12, 2017 7:59 AM
> > To: Moore, Robert ; Zheng, Lv
> > ; Wysocki, Rafael J ;
> > l...@kernel.org; linux-a...@vger.kernel.org; de...@acpica.org; linux-
> > ker...@vger.kernel.org
> > Cc: vadim.lomovt...@cavium.com
> > Subject: [PATCH] acpi: acpica: add acpi status check prior walking
> > through namespace
> >
> > From: Vadim Lomovtsev
> >
> > While having kernel built with ACPI support enabled and booted over FDT,
> > the ltp_acpi test from LTP suite causes kernel crash while calling
> > acpi_ns_walk_namespace(). The acpi_get_devices is high level wrapper for
> > it, so we need to protect kernel from crashes by adding acpi status
> > check before walking through namespace which is not loaded because of
> > acpi is disabled.
> >
> > Signed-off-by: Vadim Lomovtsev
> > ---
> > drivers/acpi/acpica/nsxfeval.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/acpi/acpica/nsxfeval.c
> > b/drivers/acpi/acpica/nsxfeval.c index 783f4c8..e0eb9ae 100644
> > --- a/drivers/acpi/acpica/nsxfeval.c
> > +++ b/drivers/acpi/acpica/nsxfeval.c
> > @@ -52,6 +52,8 @@
> > #define _COMPONENT ACPI_NAMESPACE
> > ACPI_MODULE_NAME("nsxfeval")
> >
> > +extern int acpi_disabled;
> > +
> > /* Local prototypes */
> > static void acpi_ns_resolve_references(struct acpi_evaluate_info
> > *info);
> >
> > @@ -812,6 +814,11 @@ static void acpi_ns_resolve_references(struct
> > acpi_evaluate_info *info)
> >
> > ACPI_FUNCTION_TRACE(acpi_get_devices);
> >
> > + /* check if ACPI disabled to prevent kernel crash later */
> > + if (acpi_disabled) {
> > + return_ACPI_STATUS(AE_NOT_CONFIGURED);
> > + }
> > +
> > /* Parameter validation */
> >
> > if (!user_function) {
> > --
> > 1.8.3.1
>
RE: [PATCH] acpi: acpica: add acpi status check prior walking through namespace
Another way to look at this is that the kernel should not be calling ACPI
interfaces if ACPI is disabled.
> -Original Message-
> From: Vadim Lomovtsev [mailto:vadim.lomovt...@caviumnetworks.com]
> Sent: Tuesday, December 12, 2017 7:59 AM
> To: Moore, Robert ; Zheng, Lv
> ; Wysocki, Rafael J ;
> l...@kernel.org; linux-a...@vger.kernel.org; de...@acpica.org; linux-
> ker...@vger.kernel.org
> Cc: vadim.lomovt...@cavium.com
> Subject: [PATCH] acpi: acpica: add acpi status check prior walking
> through namespace
>
> From: Vadim Lomovtsev
>
> While having kernel built with ACPI support enabled and booted over FDT,
> the ltp_acpi test from LTP suite causes kernel crash while calling
> acpi_ns_walk_namespace(). The acpi_get_devices is high level wrapper for
> it, so we need to protect kernel from crashes by adding acpi status
> check before walking through namespace which is not loaded because of
> acpi is disabled.
>
> Signed-off-by: Vadim Lomovtsev
> ---
> drivers/acpi/acpica/nsxfeval.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/acpi/acpica/nsxfeval.c
> b/drivers/acpi/acpica/nsxfeval.c index 783f4c8..e0eb9ae 100644
> --- a/drivers/acpi/acpica/nsxfeval.c
> +++ b/drivers/acpi/acpica/nsxfeval.c
> @@ -52,6 +52,8 @@
> #define _COMPONENT ACPI_NAMESPACE
> ACPI_MODULE_NAME("nsxfeval")
>
> +extern int acpi_disabled;
> +
> /* Local prototypes */
> static void acpi_ns_resolve_references(struct acpi_evaluate_info
> *info);
>
> @@ -812,6 +814,11 @@ static void acpi_ns_resolve_references(struct
> acpi_evaluate_info *info)
>
> ACPI_FUNCTION_TRACE(acpi_get_devices);
>
> + /* check if ACPI disabled to prevent kernel crash later */
> + if (acpi_disabled) {
> + return_ACPI_STATUS(AE_NOT_CONFIGURED);
> + }
> +
> /* Parameter validation */
>
> if (!user_function) {
> --
> 1.8.3.1
