[PATCH V2] efi: efibc: Guard against allocation failure

[PATCH V2] efi: efibc: Guard against allocation failure

[Guilherme G. Piccoli]

There is a single kmalloc in this driver, and it's not currently
guarded against allocation failure. Do it here by just bailing-out
the reboot handler, in case this tentative allocation fails.

Fixes: 416581e48679 ("efi: efibc: avoid efivar API for setting variables")
Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
---

V2:
* Rebased against 6.0-rc4;
* Dropped from the original series [0].

[0] https://lore.kernel.org/linux-efi/20220729194532.228403-1-gpiccoli@igalia.com/


 drivers/firmware/efi/efibc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/firmware/efi/efibc.c b/drivers/firmware/efi/efibc.c
index 8ced7af8e56d..4f9fb086eab7 100644
--- a/drivers/firmware/efi/efibc.c
+++ b/drivers/firmware/efi/efibc.c
@@ -48,6 +48,9 @@ static int efibc_reboot_notifier_call(struct notifier_block *notifier,
 		return NOTIFY_DONE;
 
 	wdata = kmalloc(MAX_DATA_LEN * sizeof(efi_char16_t), GFP_KERNEL);
+	if (!wdata)
+		return NOTIFY_DONE;
+
 	for (l = 0; l < MAX_DATA_LEN - 1 && str[l] != '\0'; l++)
 		wdata[l] = str[l];
 	wdata[l] = L'\0';
-- 
2.37.2

Re: [PATCH V2] efi: efibc: Guard against allocation failure

[Christophe JAILLET]

Le 09/09/2022 à 21:42, Guilherme G. Piccoli a écrit :
> There is a single kmalloc in this driver, and it's not currently
> guarded against allocation failure. Do it here by just bailing-out
> the reboot handler, in case this tentative allocation fails.
> 
> Fixes: 416581e48679 ("efi: efibc: avoid efivar API for setting variables")
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
> ---
> 
> V2:
> * Rebased against 6.0-rc4;
> * Dropped from the original series [0].
> 
> [0] https://lore.kernel.org/linux-efi/20220729194532.228403-1-gpiccoli@igalia.com/
> 
> 
>   drivers/firmware/efi/efibc.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/firmware/efi/efibc.c b/drivers/firmware/efi/efibc.c
> index 8ced7af8e56d..4f9fb086eab7 100644
> --- a/drivers/firmware/efi/efibc.c
> +++ b/drivers/firmware/efi/efibc.c
> @@ -48,6 +48,9 @@ static int efibc_reboot_notifier_call(struct notifier_block *notifier,
>   		return NOTIFY_DONE;
>   
>   	wdata = kmalloc(MAX_DATA_LEN * sizeof(efi_char16_t), GFP_KERNEL);
Hi,

even if mostly useless in this case, kmalloc_array()?

Or certainly maybe even better, kstrndup()?

CJ

> +	if (!wdata)
> +		return NOTIFY_DONE;
> +
>   	for (l = 0; l < MAX_DATA_LEN - 1 && str[l] != '\0'; l++)
>   		wdata[l] = str[l];
>   	wdata[l] = L'\0';

Re: [PATCH V2] efi: efibc: Guard against allocation failure

[Guilherme G. Piccoli]

On 10/09/2022 01:56, Christophe JAILLET wrote:
> [...]
>>   	wdata = kmalloc(MAX_DATA_LEN * sizeof(efi_char16_t), GFP_KERNEL);
> Hi,
> 
> even if mostly useless in this case, kmalloc_array()?
> 
> Or certainly maybe even better, kstrndup()?
> 
> CJ
> 

Thanks! It's up to Ard, I could rework with this change if makes sense.
Cheers,


Guilherme

Re: [PATCH V2] efi: efibc: Guard against allocation failure

[Ard Biesheuvel]

On Sun, 11 Sept 2022 at 16:36, Guilherme G. Piccoli <gpiccoli@igalia.com> wrote:
>
> On 10/09/2022 01:56, Christophe JAILLET wrote:
> > [...]
> >>      wdata = kmalloc(MAX_DATA_LEN * sizeof(efi_char16_t), GFP_KERNEL);
> > Hi,
> >
> > even if mostly useless in this case, kmalloc_array()?
> >
> > Or certainly maybe even better, kstrndup()?
> >
> > CJ
> >
>
> Thanks! It's up to Ard, I could rework with this change if makes sense.
> Cheers,
>

kstrndup() does not work on wide strings so I think the code is fine as is.

I've queued it as a fix - thanks.

Re: [PATCH V2] efi: efibc: Guard against allocation failure

[Guilherme G. Piccoli]

On 20/09/2022 13:44, Ard Biesheuvel wrote:
> [...]
> I've queued it as a fix - thanks.

Thanks Ard!