Bug 83721

Summary:	gadgetfs regression: module refcount bug since 3.10
Product:	Drivers	Reporter:	S. Lockwood-Childs (sjl)
Component:	USB	Assignee:	Greg Kroah-Hartman (greg)
Status:	NEW ---
Severity:	normal	CC:	balbi, felipebalbi
Priority:	P1
Hardware:	All
OS:	Linux
Kernel Version:	3.10 - present (3.17-rc3)	Subsystem:
Regression:	Yes	Bisected commit-id:
Attachments:	fixes module refcount bug fixes module refcount bug (reference original commit in comment and added Cc stable tag)

Description S. Lockwood-Childs 2014-09-02 02:20:59 UTC

Created attachment 149001 [details]
fixes module refcount bug

gadgetfs has had a module refcount bug in all recent kernels, from v3.10 up
through present. I realize that gadgetfs is deprecated in favor of functionfs
nowadays, but probably you will want to fix it anyways since gadgetfs is still
in the tree.

The refcount bug can be reproduced with the following test case (tried on both
ARM and x86-64):

* build gadgetfs as module, build gadgetfs example app from
  http://www.linux-usb.org/gadget/usb.c

* run test script

  modprobe dummy_hcd
  modprobe gadgetfs
  mkdir -p /dev/gadget
  mount -t gadgetfs none /dev/gadget 
  lsmod | grep gadgetfs # this will show usage 1, as expected
  ./usb
  lsmod | grep gadgetfs # still shows usage 1

* now when example app is killed, gadgetfs usage count
  goes from 1 down to 0 (despite still being mounted)

* in fact, the usage count is decremented every time a process exits 
  after using gadgetfs (meaning it goes negative on the 2nd exit)

Some poking around showed fops_put() in __fput() being the guilty party for
decrementing the refcount on process exit, which was an unbalanced module put
because running the process didn't do a module get.

Reverting the gadgetfs part of commit 3273097ee9c077 "gadgetfs: don't bother with fops->owner" makes building gadgetfs as module work again, with balanced module gets and puts when a process uses gadgetfs then exits.

Comment 1 Greg Kroah-Hartman 2014-09-02 02:30:17 UTC

On Tue, Sep 02, 2014 at 02:20:59AM +0000, bugzilla-daemon@bugzilla.kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=83721
> 
>             Bug ID: 83721
>            Summary: gadgetfs regression: module refcount bug since 3.10

Please send to the linux-usb@vger.kernel.org mailing list.

Comment 2 S. Lockwood-Childs 2014-09-02 04:58:56 UTC

Already done, forgot to include the link here
http://thread.gmane.org/gmane.linux.usb.general/113763

Comment 3 S. Lockwood-Childs 2014-10-01 06:11:23 UTC

Are there any other proposed solutions for this regression (besides the suggested revert https://bugzilla.kernel.org/attachment.cgi?id=149001) I could help test?

Comment 4 Felipe Balbi 2014-10-02 01:20:19 UTC

(In reply to S. Lockwood-Childs from comment #3)
> Are there any other proposed solutions for this regression (besides the
> suggested revert https://bugzilla.kernel.org/attachment.cgi?id=149001) I
> could help test?

That's not exactly how you provide a revert. You need to do an actual git revert, add Cc: <stable@vger.kernel.org> # $first-known-broken-release, and so on.

Also, please make sure to Cc proper folks on the mailing list. scripts/get_maintainer.pl helps with that, I'll wait for a new version and will pick it up for v3.18-rc cycle.

cheers

Comment 5 Felipe Balbi 2014-10-02 01:38:59 UTC

Alright, so the commit which you claim is wrong is commit 3273097ee9c077512bcb017535b0781f1e1f7a6d. When sending your patch, make sure to Cc him as well as me :-)

Comment 6 S. Lockwood-Childs 2014-10-20 07:05:57 UTC

By "Reverting the gadgetfs part of commit 3273097ee9c077" I meant that it isn't a full revert; there is an equivalent change to functionfs that didn't result in the same breakage, so it seems like that part should be left in. Thus I don't think it can be done as an actual git revert.

About CC-ing linux-usb mailing list and all those other folks from get_maintainer on this bug, that seems to be easier said than done -- I tried CC-ing linux-usb when entering the bug initially, and bugzilla claimed not to know the address. It did the same thing when I tried to add your email from MAINTAINERS, so eventually I did some web searches to figure out what email you had used on other bugs. 

At least you threw me one softball, the request to add Cc stable tag to the patch ;)

Comment 7 S. Lockwood-Childs 2014-10-20 07:13:49 UTC

Created attachment 154231 [details]
fixes module refcount bug (reference original commit in comment and added Cc stable tag)