linux-kernel - Re: [PATCH] ipmi: Fix memleaking for add

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]

Message-id: <4C4E3F03.4030707@acm.org>
Date:	Mon, 26 Jul 2010 21:05:55 -0500
From:	Corey Minyard <minyard@....org>
To:	Yinghai Lu <yinghai@...nel.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Matthew Garrett <mjg@...hat.com>,
	Len Brown <len.brown@...el.com>,
	Myron Stowe <myron.stowe@...com>,
	openipmi-developer@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ipmi: Fix memleaking for add_smi when duplicating happen

Please run this through checkpatch, as it has coding style violations.

Also, this patch appears to fix bugs in addition to adding the print.  
Can we have a separate patch for that?

I'm also not clear on the reason for this.  I believe all this 
information is already available in /proc/ipmi/<if#>/params.  I don't 
think there is a strong reason to print it to the log.

-corey

On 07/26/2010 08:25 PM, Yinghai Lu wrote:
> also print out the reg spacing and size for spmi and smbios.
> so bios guys could have idea to make them consistent.
>
> Signed-off-by: Yinghai Lu<yinghai@...nel.org>
>
> ---
>   drivers/char/ipmi/ipmi_si_intf.c |   47 ++++++++++++++++++++++++++++++++-------
>   1 file changed, 39 insertions(+), 8 deletions(-)
>
> Index: linux-2.6/drivers/char/ipmi/ipmi_si_intf.c
> ===================================================================
> --- linux-2.6.orig/drivers/char/ipmi/ipmi_si_intf.c
> +++ linux-2.6/drivers/char/ipmi/ipmi_si_intf.c
> @@ -1804,9 +1804,12 @@ static int hotmod_handler(const char *va
>   				info->irq_setup = std_irq_setup;
>   			info->slave_addr = ipmb;
>
> -			if (!add_smi(info))
> +			if (!add_smi(info)) {
>   				if (try_smi_init(info))
>   					cleanup_one_si(info);
> +			} else {
> +				kfree(info);
> +			}
>   		} else {
>   			/* remove */
>   			struct smi_info *e, *tmp_e;
> @@ -1890,9 +1893,12 @@ static __devinit void hardcode_find_bmc(
>   			info->irq_setup = std_irq_setup;
>   		info->slave_addr = slave_addrs[i];
>
> -		if (!add_smi(info))
> +		if (!add_smi(info)) {
>   			if (try_smi_init(info))
>   				cleanup_one_si(info);
> +		} else {
> +			kfree(info);
> +		}
>   	}
>   }
>
> @@ -2088,7 +2094,13 @@ static __devinit int try_init_spmi(struc
>   	}
>   	info->io.addr_data = spmi->addr.address;
>
> -	add_smi(info);
> +	pr_info("ipmi_si: SPMI: %s %#lx regsize %d spacing %d irq %d\n",
> +		 (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem",
> +		 info->io.addr_data, info->io.regsize, info->io.regspacing,
> +		 info->irq);
> +
> +	if (add_smi(info))
> +		kfree(info);
>
>   	return 0;
>   }
> @@ -2204,7 +2216,10 @@ static int __devinit ipmi_pnp_probe(stru
>   		 res, info->io.regsize, info->io.regspacing,
>   		 info->irq);
>
> -	return add_smi(info);
> +	if (add_smi(info))
> +		goto err_free;
> +
> +	return 0;
>
>   err_free:
>   	kfree(info);
> @@ -2362,7 +2377,13 @@ static __devinit void try_init_dmi(struc
>   	if (info->irq)
>   		info->irq_setup = std_irq_setup;
>
> -	add_smi(info);
> +	pr_info("ipmi_si: SMBIOS: %s %#lx regsize %d spacing %d irq %d\n",
> +		 (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem",
> +		 info->io.addr_data, info->io.regsize, info->io.regspacing,
> +		 info->irq);
> +
> +	if (add_smi(info))
> +		kfree(info);
>   }
>
>   static void __devinit dmi_find_bmc(void)
> @@ -2468,7 +2489,10 @@ static int __devinit ipmi_pci_probe(stru
>   		&pdev->resource[0], info->io.regsize, info->io.regspacing,
>   		info->irq);
>
> -	return add_smi(info);
> +	if (add_smi(info))
> +		kfree(info);
> +
> +	return 0;
>   }
>
>   static void __devexit ipmi_pci_remove(struct pci_dev *pdev)
> @@ -2581,7 +2605,12 @@ static int __devinit ipmi_of_probe(struc
>
>   	dev_set_drvdata(&dev->dev, info);
>
> -	return add_smi(info);
> +	if (add_smi(info)) {
> +		kfree(info);
> +		return -EBUSY;
> +	}
> +
> +	return 0;
>   }
>
>   static int __devexit ipmi_of_remove(struct of_device *dev)
> @@ -3018,6 +3047,8 @@ static __devinit void default_find_bmc(v
>   				info->io.addr_data);
>   			} else
>   				cleanup_one_si(info);
> +		} else {
> +			kfree(info);
>   		}
>   	}
>   }
> @@ -3045,7 +3076,7 @@ static int add_smi(struct smi_info *new_
>   			si_to_str[new_smi->si_type]);
>   	mutex_lock(&smi_infos_lock);
>   	if (!is_new_interface(new_smi)) {
> -		printk(KERN_CONT PFX "duplicate interface\n");
> +		printk(KERN_CONT " duplicate interface\n");
>   		rv = -EBUSY;
>   		goto out_err;
>   	}
>    

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/