linux-kernel - Re: [PATCH] ACPI: Correct and clean up the logic of acpi_parse_entries

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] [day] [month] [year] [list]

Date:   Wed, 6 Dec 2023 20:47:20 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Dave Jiang <dave.jiang@...el.com>, Yuntao Wang <ytcoode@...il.com>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Len Brown <lenb@...nel.org>, linux-acpi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ACPI: Correct and clean up the logic of acpi_parse_entries_array()

On Wed, Dec 6, 2023 at 6:02 PM Dave Jiang <dave.jiang@...el.com> wrote:
>
>
>
> On 12/6/23 09:42, Rafael J. Wysocki wrote:
> > On Mon, Nov 20, 2023 at 12:42 PM Yuntao Wang <ytcoode@...il.com> wrote:
> >>
> >> The original intention of acpi_parse_entries_array() is to return the
> >> number of all matching entries on success. This number may be greater than
> >> the value of the max_entries parameter. When this happens, the function
> >> will output a warning message, indicating that `count - max_entries`
> >> matching entries remain unprocessed and have been ignored.
> >>
> >> However, commit 4ceacd02f5a1 ("ACPI / table: Always count matched and
> >> successfully parsed entries") changed this logic to return the number of
> >> entries successfully processed by the handler. In this case, when the
> >> max_entries parameter is not zero, the number of entries successfully
> >> processed can never be greater than the value of max_entries. In other
> >> words, the expression `count > max_entries` will always evaluate to false.
> >> This means that the logic in the final if statement will never be executed.
> >>
> >> Commit 99b0efd7c886 ("ACPI / tables: do not report the number of entries
> >> ignored by acpi_parse_entries()") mentioned this issue, but it tried to fix
> >> it by removing part of the warning message. This is meaningless because the
> >> pr_warn statement will never be executed in the first place.
> >>
> >> Commit 8726d4f44150 ("ACPI / tables: fix acpi_parse_entries_array() so it
> >> traverses all subtables") introduced an errs variable, which is intended to
> >> make acpi_parse_entries_array() always traverse all of the subtables,
> >> calling as many of the callbacks as possible. However, it seems that the
> >> commit does not achieve this goal. For example, when a handler returns an
> >> error, none of the handlers will be called again in the subsequent
> >> iterations. This result appears to be no different from before the change.
> >>
> >> This patch corrects and cleans up the logic of acpi_parse_entries_array(),
> >> making it return the number of all matching entries, rather than the number
> >> of entries successfully processed by handlers. Additionally, if an error
> >> occurs when executing a handler, the function will return -EINVAL immediately.
> >>
> >> This patch should not affect existing users of acpi_parse_entries_array().
> >>
> >> Signed-off-by: Yuntao Wang <ytcoode@...il.com>
> >
> > This needs to be ACKed by Dave Jiang or Dan Williams.
>
> Reviewed-by: Dave Jiang <dave.jiang@...el.com>

Applied as 6.8 material, thanks!