NACK: [SRU][J/N/Q/R:Unstable][PATCH 1/1] UBUNTU: SAUCE: efi: Fix swapped arguments to bsearch() in efi_status_to_*()

[Abdur Rahman]

On 2/10/26 12:14 AM, Dongdong Tao wrote:
> BugLink: https://bugs.launchpad.net/bugs/2141276
>
> The bsearch() function signature is:
>    void *bsearch(const void *key, const void *base,
>                  size_t nmemb, size_t size,
>                  int (*compar)(const void *, const void *));
>
> The third argument is the number of elements (nmemb), and the fourth
> is the size of each element. However, in efi_status_to_err() and
> efi_status_to_str(), these arguments were passed in the wrong order:
> sizeof(struct efi_error_code) was passed as nmemb, and num (the actual
> count) was passed as size.
>
> This bug causes bsearch to calculate incorrect element offsets, reading
> at every 12 bytes instead of every 24 bytes (on 64-bit), potentially
> returning incorrect results or failing to find valid status codes.
>
> The bug was introduced in the SAUCE patch:
>    "UBUNTU: SAUCE: (lockdown) Add efi_status_to_str() and rework
>     efi_status_to_err()."
>
> which was cherry-picked from kernel-ark commit 2ae9082db0b5.
>
> (backported from commit 49bcc48074ba1f9c772b5c7ae11123a8ed3e0ac1
>   https://gitlab.com/cki-project/kernel-ark)
>
> Signed-off-by: Dongdong Tao <dongdong.tao at canonical.com>
> ---
>   drivers/firmware/efi/efi.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 715b736be77a..3f0451709fbf 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -930,7 +930,7 @@ int efi_status_to_err(efi_status_t status)
>   	size_t num = sizeof(efi_error_codes) / sizeof(struct efi_error_code);
>   
>   	found = bsearch((void *)(uintptr_t)status, efi_error_codes,
> -			sizeof(struct efi_error_code), num,
> +			num, sizeof(struct efi_error_code),
>   			efi_status_cmp_bsearch);
>   	if (!found)
>   		return -EINVAL;
> @@ -944,7 +944,7 @@ efi_status_to_str(efi_status_t status)
>   	size_t num = sizeof(efi_error_codes) / sizeof(struct efi_error_code);
>   
>   	found = bsearch((void *)(uintptr_t)status, efi_error_codes,
> -			sizeof(struct efi_error_code), num,
> +			num, sizeof(struct efi_error_code),
>   			efi_status_cmp_bsearch);
>   	if (!found)
>   		return "Unknown error code";

Thank you for submitting this patch. It seems that the patches apply 
cleanly
to the kernels and `cherry-picked from` needs to be used instead of 
`backported
from` in the provenance.

It is also desirable to have the complete upstream commit provenance 
included
in the cherry-pick/backport.

See: https://wiki.ubuntu.com/Kernel/Dev/StablePatchFormat

Nacked-by: Abdur Rahman <abdur.rahman at canonical.com>