Bug 480211

Summary: race between procfile opens and setting of module owner
Product: [Fedora] Fedora Reporter: Jeff Layton <jlayton>
Component: kernelAssignee: Jeff Layton <jlayton>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: low Docs Contact:
Priority: low    
Version: rawhideCC: kernel-maint, nhorman, quintela, rwheeler, staubach, steved
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-04-06 11:38:52 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Jeff Layton 2009-01-15 18:20:43 UTC
This problem was originally noticed by Neil Horman in RHEL4, but the problem still exists upstream and is fairly widespread.

When a procfile is created, one thing that can be done is to set the "owner" field in the proc_dir_entry. When this is done, a module_get is done against that module when the file is opened and put when it's closed. The problem is that there is a race window where the procfile exists on the system but the owner is not yet set.

If this happens then no module reference will be taken on open
(try_module_get(NULL) is a no-op that returns success). If the owner is then set while the file is open a module reference will be put when it's closed. This will make the module refcount go negative.

I believe fixing this requires that we make certain that if the owner is to be set, that it be set when the proc_dir_entry is created but before proc_register (similar to how proc_create sets the fops).

This is not a difficult problem to fix, but it will probably be labour intensive. A new function will need to be created that can create the procfile with the owner already set and the places where we create procfiles will need to be fixed to use it (and to pass in an "owner" arg).

Some possibilities for shortcuts:

1) if the owner field in the file_operations struct is set then set pde->owner to that. We'll have to audit the existing uses to make sure that this doesn't break anything

2) turn proc_create and proc_create_data into wrappers that call the new function with an owner of THIS_MODULE. That would probably work, but may mean that some places that don't set the owner would now do so.

I count >300 places that call either proc_create, proc_create_data, or create_proc_entry. Some sort of shortcut may be a necessity...

Comment 1 Jeff Layton 2009-01-28 19:11:44 UTC
Alexey Dobriyan took the upstream BZ and proposed a patchset for this on LKML a couple of days ago. He essentially is going to remove the 'owner' field altogether. I've gone ahead and Acked the set after testing it. With luck, this will make 2.6.30.

Comment 2 Jeff Layton 2009-03-02 14:22:28 UTC
Looks like this is in linux-next and the /proc tree. 2.6.30 hasn't yet opened, but I'm hopeful that this will make it.

Comment 3 Jeff Layton 2009-03-31 00:04:41 UTC
Looks like Alexey sent the pull request to Linus today, so this should make 2.6.30.

Comment 4 Jeff Layton 2009-04-06 11:38:52 UTC
Patchset is now in Linus' tree, I'll close this with a resolution of rawhide. If it's not there now then it will be soon.