powerpc/pasemi: Add a null pointer check to the pas_setup_mce_regs

[PATCH] powerpc/pasemi: Add a null pointer check to the pas_setup_mce_regs

Posted by Kunwu Chan 1 year, 11 months ago

kasprintf() returns a pointer to dynamically allocated memory
which can be NULL upon failure. Ensure the allocation was successful
by checking the pointer validity.

Signed-off-by: Kunwu Chan <chentao@kylinos.cn>
---
 arch/powerpc/platforms/pasemi/setup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/platforms/pasemi/setup.c b/arch/powerpc/platforms/pasemi/setup.c
index 0761d98e5be3..8f98f3b58888 100644
--- a/arch/powerpc/platforms/pasemi/setup.c
+++ b/arch/powerpc/platforms/pasemi/setup.c
@@ -165,6 +165,8 @@ static int __init pas_setup_mce_regs(void)
 	while (dev && reg < MAX_MCE_REGS) {
 		mce_regs[reg].name = kasprintf(GFP_KERNEL,
 						"mc%d_mcdebug_errsta", reg);
+		if (!mce_regs[reg].name)
+			return -ENOMEM;
 		mce_regs[reg].addr = pasemi_pci_getcfgaddr(dev, 0x730);
 		dev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa00a, dev);
 		reg++;
-- 
2.39.2

Re: [PATCH] powerpc/pasemi: Add a null pointer check to the pas_setup_mce_regs

Posted by Christophe Leroy 3 months, 2 weeks ago


Le 17/01/2024 à 10:17, Kunwu Chan a écrit :
> kasprintf() returns a pointer to dynamically allocated memory
> which can be NULL upon failure. Ensure the allocation was successful
> by checking the pointer validity.
> 
> Signed-off-by: Kunwu Chan <chentao@kylinos.cn>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/platforms/pasemi/setup.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/pasemi/setup.c b/arch/powerpc/platforms/pasemi/setup.c
> index 0761d98e5be3..8f98f3b58888 100644
> --- a/arch/powerpc/platforms/pasemi/setup.c
> +++ b/arch/powerpc/platforms/pasemi/setup.c
> @@ -165,6 +165,8 @@ static int __init pas_setup_mce_regs(void)
>   	while (dev && reg < MAX_MCE_REGS) {
>   		mce_regs[reg].name = kasprintf(GFP_KERNEL,
>   						"mc%d_mcdebug_errsta", reg);
> +		if (!mce_regs[reg].name)
> +			return -ENOMEM;

I think you will leak the reference taken with last call to 
pci_get_device() here. I think we need a call to pci_dev_put(dev) before 
bailing out.

By the way there will also be the same leak if the while loop ends when 
reg reaches MAX_MCE_REGS while dev is still not NULL.


>   		mce_regs[reg].addr = pasemi_pci_getcfgaddr(dev, 0x730);
>   		dev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa00a, dev);
>   		reg++;