linux-kernel - Re: [PATCH] ACPI: OSL: Initialize output value

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 21 Nov 2023 21:05:49 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Nikita Kiryushin <kiryushin@...ud.ru>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>, Lin Ming <ming.m.lin@...el.com>,
        Bob Moore <robert.moore@...el.com>, linux-acpi@...r.kernel.org,
        linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org
Subject: Re: [PATCH] ACPI: OSL: Initialize output value

On Fri, Nov 10, 2023 at 8:45 PM Nikita Kiryushin <kiryushin@...ud.ru> wrote:
>
> Buffer variable for result (value32) is not initialized.
> This can lead to unexpected behavior, if underlying platform-specific
> raw_pci_read fails to report error (uninitialized value will be treated
> as valid pci-read result), or exposition of unexpected data to PCI
> config space reader.
>
> Zeroing of buffer value addresses the later problem and makes the
> behavior in the former case somewhat more predictable.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: c5f0231ee6b0 ("ACPICA: Fix acpi_os_read_pci_configuration prototype")
> Signed-off-by: Nikita Kiryushin <kiryushin@...ud.ru>
> ---
>   drivers/acpi/osl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index c09cc3c68633..d3c0f7f01a29 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -788,7 +788,7 @@ acpi_os_read_pci_configuration(struct acpi_pci_id
> *pci_id, u32 reg,
>                                u64 *value, u32 width)
>   {
>         int result, size;
> -       u32 value32;
> +       u32 value32 = 0U;

So wouldn't it be better to avoid modifying *value at all if
raw_pci_read() returns an error?

And if it returns a success, why wouldn't it be trusted?

>         if (!value)
>                 return AE_BAD_PARAMETER;
> --