Summary:	DAHLIN-00226: [patch] dahdi-base locks channel-exporting module in kernel if channel open fails
Reporter:	Angelos Varvitsiotis (avarvit)	Labels:
Date Opened:	2010-12-03 23:21:17.000-0600	Date Closed:	2011-01-20 23:31:27.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	dahdi (the module)
Versions:	2.3.0.1	Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 0001-dahdi-Prevent-unloadable-module-on-failed-open.patch
Description:	If chan->span->ops->open() fails, then DAHDI_FLAGBIT_OPEN never gets set and module_put is never called on the exporter module. ****** ADDITIONAL INFORMATION ****** Have not checked on 2.3.0, but the problem likely exists there too. I *have* checked 2.4.0 and the problem has not been fixed on that, neither (apparently nobody's channels ever fail to open() except mine :-).
Comments:	By: Shaun Ruffell (sruffell) 2010-12-04 09:09:41.000-0600 avarit: Thanks for opening the issue.  I had to delete the code from the description since the project needs a contributors license agreement from contributors for all patches. https://issues.asterisk.org/view_license_agreement.php Do you want to attach a patch to the issue?  I think there is enough information left in the description to act if you are not planning on doing that. Thanks again. By: Angelos Varvitsiotis (avarvit) 2010-12-05 07:51:33.000-0600 Hi sruffell, Thanks for responding and for taking appropriate action. My userid is avarvit, not avarit. I have now signed the license, so I guess I am allowed to (re)post the patch without anyone having to take it down for legal reasons. Here it is: (patch removed by sruffell) By: Shaun Ruffell (sruffell) 2010-12-05 09:54:50.000-0600 avarvit: sorry about misspelling your username. I did have to remove it again (sorry).   All code submissions need to be updated via the "Upload File" block on the issue, and then select the "this is a code or documentation submission". You won't be able to upload until after your license submission has been reviewed which will most likely be on Monday. By: Angelos Varvitsiotis (avarvit) 2010-12-05 23:16:05.000-0600 OK then, I guess the following fix instructions do not fall under any reasonable definition of "code", so you will not have to kill them (now three times in a row): the suggested fix is, around line 2714, check the return value from the open function; if that value is non-zero, then invoke module_put, because in that execution path module_put is not called anywhere else and thus the exporting module remains locked in the kernel. By: Shaun Ruffell (sruffell) 2010-12-06 11:13:26.000-0600 avarvit: I've attached a potential patch against the current trunk.  Mind if I put your real name / email address in it?   Actually, I just checked and it looks like your license has been approved.  So you could upload your patch with the "Upload File" button.   Not required, but if you use git to make your patch, then I can use (mostly) your commit message. But if you don't want to fool with it, I'll commit the patch that I just attached. By: Angelos Varvitsiotis (avarvit) 2010-12-06 22:48:52.000-0600 Hi sruffel, I took a look at your patch (without trying it) and it looks good. It seems that, in comparison to what I originally posted, you have just added a temp variable "ops" to hold the channel's span ops; other than that, the patch looks identical to my originally posted code. There is no need for me to submit a patch of "my own". Yes, it's fine if you put my real name and email in the patch you have produced. Thanks for taking immediate action on the issue! By: Digium Subversion (svnbot) 2010-12-07 08:20:39.000-0600 Repository: dahdi Revision: 9510 U   linux/trunk/drivers/dahdi/dahdi-base.c ------------------------------------------------------------------------ r9510 | sruffell | 2010-12-07 08:20:39 -0600 (Tue, 07 Dec 2010) | 13 lines dahdi: Prevent unloadable module on failed open. If chan->span->ops->open() fails then the reference count of the module implementing the board driver will not be decremented.  The result is a module that would always be "in use" and unloadable. This change makes sure to release that reference when open failed. (closes issue DAHLIN-226) Reported by: avarvit Signed-off-by: Shaun Ruffell <sruffell@digium.com> Acked-by: Angelos Varvitsiotis <avarvit@admin.grnet.gr> ------------------------------------------------------------------------ http://svn.digium.com/view/dahdi?view=rev&revision=9510 By: Digium Subversion (svnbot) 2010-12-13 08:57:15.000-0600 Repository: dahdi Revision: 9530 U   linux/trunk/drivers/dahdi/dahdi-base.c ------------------------------------------------------------------------ r9530 | sruffell | 2010-12-13 08:57:13 -0600 (Mon, 13 Dec 2010) | 11 lines dahdi: Do not dereference chan->span for pseudo channels. Fixes a regression introduced in r9510 which saves a pointer to the ops member of a channel's span before checking if the channel is a pseudo. Psuedo channels do not have spans. (issue DAHLIN-227) (issue DAHLIN-226) Reported by: alecdavis Signed-off-by: Shaun Ruffell <sruffell@digium.com> ------------------------------------------------------------------------ http://svn.digium.com/view/dahdi?view=rev&revision=9530 By: Digium Subversion (svnbot) 2011-01-20 23:31:27.000-0600 Repository: dahdi Revision: 9682 U   linux/branches/2.4/drivers/dahdi/dahdi-base.c ------------------------------------------------------------------------ r9682 | sruffell | 2011-01-20 23:31:26 -0600 (Thu, 20 Jan 2011) | 15 lines dahdi: Prevent unloadable module on failed open. If chan->span->ops->open() fails then the reference count of the module implementing the board driver will not be decremented.  The result is a module that would always be "in use" and unloadable. This change makes sure to release that reference when open failed. (closes issue DAHLIN-226) Reported by: avarvit Signed-off-by: Shaun Ruffell <sruffell@digium.com> Acked-by: Angelos Varvitsiotis <avarvit@admin.grnet.gr> Origin: http://svnview.digium.com/svn/dahdi?view=rev&rev=9510 ------------------------------------------------------------------------ http://svn.digium.com/view/dahdi?view=rev&revision=9682