linux-kernel - Re: [PATCH v2] fusion-mptbase: handle failed allocation for workqueue

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: <1455814515.22112.476.camel@localhost.localdomain>
Date:	Thu, 18 Feb 2016 11:55:15 -0500
From:	Ewan Milne <emilne@...hat.com>
To:	Johannes Thumshirn <jthumshirn@...e.de>, thenzl@...hat.com
Cc:	thenzl@...hat.com, Insu Yun <wuninsu@...il.com>,
	nagalakshmi.nandigama@...gotech.com,
	praveen.krishnamoorthy@...gotech.com,
	sreekanth.reddy@...gotech.com, abhijit.mahajan@...gotech.com,
	MPT-FusionLinux.pdl@...gotech.com, linux-scsi@...r.kernel.org,
	linux-kernel@...r.kernel.org, taesoo@...ech.edu,
	yeongjin.jang@...ech.edu, insu@...ech.edu, changwoo@...ech.edu
Subject: Re: [PATCH v2] fusion-mptbase: handle failed allocation for
 workqueue

On Thu, 2016-02-18 at 11:40 -0500, Ewan Milne wrote:
> It also appears to me upon further inspection that the existing code has
> other problems.  In particular, once mpt_mapresources() has returned
> with a nonzero error code, it looks like iounmap() should be called, but
> it is not in the case of a failed allocation of reset_work_q.  I'm also
> not sure why pci_release_selected_regions() is only called for the case
> of mpt_do_ioc_recovery() returning != -5 when it is called whenever
> there is a failed allocation of reset_work_q.
> 
> Consider the attached patch (untested, because I don't have hardware):
> It shows what I would do for labels & error handling.  If the rc != -5
> case of return from mpt_do_ioc_recovery() could be eliminated, then
> another label "out_free_fw_event_q:" could be added prior to the other
> error cases at the end, and all the code after the printk() in that path
> could be replaced by "goto out_free_fw_event_q;"
> 
> 	if ((r = mpt_do_ioc_recovery(ioc, MPT_HOSTEVENT_IOC_BRINGUP,
> 	    CAN_SLEEP)) != 0){
> 		printk(MYIOC_s_ERR_FMT "didn't initialize properly! (%d)\n",
> 		    ioc->name, r);
> 		goto out_free_fw_event_q;
> 	}
> ...
> 
> out_free_fw_event_q:
> 	destroy_workqueue(ioc->fw_event_q);
> 	ioc->fw_event_q = NULL;
> 
> out_remove_ioc:
> ...
> 
> However I do not know if that change is legitimate.
> 
> -Ewan

There is an error in the patch attached in my previous mail.
Please refer to the attached PATCH v2 RFC.

-Ewan


View attachment "0001-mptbase-fixup-error-handling-paths-in-mpt_attach.patch" of type "text/x-patch" (3019 bytes)