public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: ebiederm@xmission•com (Eric W. Biederman)
To: Ben Greear <greearb@candelatech•com>
Cc: "Denis V. Lunev" <den@openvz•org>,
	Patrick McHardy <kaber@trash•net>,
	Andrew Morton <akpm@linux-foundation•org>,
	netdev@vger•kernel.org, bugme-daemon@bugzilla•kernel.org,
	devzero@web•de, Robert Olsson <robert.olsson@its•uu.se>,
	Pavel Emelyanov <xemul@openvz•org>
Subject: Re: [Bugme-new] [Bug 10737] New: pktgen procfs problem
Date: Mon, 19 May 2008 18:14:15 -0700	[thread overview]
Message-ID: <m1lk257p5k.fsf@frodo.ebiederm.org> (raw)
In-Reply-To: <48321AC2.90500@candelatech.com> (Ben Greear's message of "Mon, 19 May 2008 17:26:42 -0700")

Ben Greear <greearb@candelatech•com> writes:

> Denis V. Lunev wrote:
>
>> could you consider this preliminary patch? It fixes the problem for me
>> and Pavel agrees with it.
>>
>> The problem is that module_get is called for each file opening while
>> module_put is called only when /proc inode is destroyed. So, lets put
>> module counter if we are dealing with already initialised inode.

Acked-by: "Eric W. Biederman" <ebiederm@xmission•com>


>> ------------------------------------------------------------------------
>>
>> diff --git a/fs/proc/inode.c b/fs/proc/inode.c
>> index 6f4e8dc..b08d100 100644
>> --- a/fs/proc/inode.c
>> +++ b/fs/proc/inode.c
>> @@ -425,7 +425,8 @@ struct inode *proc_get_inode(struct super_block *sb,
> unsigned int ino,
>>  			}
>>  		}
>>  		unlock_new_inode(inode);
>> -	}
>> +	} else
>> +	       module_put(de->owner);
>>  	return inode;
>>   out_ino:
>
> I just tested this and it seems to fix my problem (I applied this to 2.6.25
> kernel).

Looks good and it seems to follow the characterization.
proc_lookup -> proc_lookup_de -> proc_get_inode 

Will happen each time a dentry is created for a name/inode.
Which is much easier now with multiple instances in
proc.

It looks like this bug an old bug that has existed since

commit e9543659715602e3180f00a227bb6db34141ac41
Author: Kirill Korotaev <dev@sw•ru>
Date:   Sun Oct 30 15:02:26 2005 -0800

    [PATCH] proc: fix of error path in proc_get_inode()
    
    This patch fixes incorrect error path in proc_get_inode(), when module
    can't be get due to being unloaded.  When try_module_get() fails, this
    function puts de(!) and still returns inode with non-getted de.
    
    There are still unresolved known bugs in proc yet to be fixed:
    - proc_dir_entry tree is managed without any serialization
    - create_proc_entry() doesn't setup de->owner anyhow,
       so setting it later manually is inatomic.
    - looks like almost all modules do not care whether
       it's de->owner is set...
    
    Signed-Off-By: Denis Lunev <den@sw•ru>
    Signed-Off-By: Kirill Korotaev <dev@sw•ru>
    Signed-off-by: Andrew Morton <akpm@osdl•org>
    Signed-off-by: Linus Torvalds <torvalds@osdl•org>

Eric

  reply	other threads:[~2008-05-20  1:20 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <bug-10737-10286@http.bugzilla.kernel.org/>
2008-05-17 21:10 ` [Bugme-new] [Bug 10737] New: pktgen procfs problem Andrew Morton
2008-05-18  1:39   ` Patrick McHardy
2008-05-18  4:56     ` Andrew Morton
2008-05-18 12:06       ` Patrick McHardy
2008-05-18 13:24         ` Patrick McHardy
2008-05-18 15:31           ` Patrick McHardy
2008-05-19  7:54             ` Denis V. Lunev
2008-05-19 10:34               ` Patrick McHardy
2008-05-20  0:26               ` Ben Greear
2008-05-20  1:14                 ` Eric W. Biederman [this message]
2008-05-20  8:25                   ` Robert Olsson
2008-05-19 21:34             ` Eric W. Biederman
2008-05-19 22:19               ` Ben Greear
2008-05-19 15:19 Alexey Dobriyan
  -- strict thread matches above, loose matches on Subject: below --
2008-05-19 16:03 Alexey Dobriyan
2008-05-19 22:21 Alexey Dobriyan
2008-05-19 22:45 ` Robert Olsson
2008-05-20 21:57 Alexey Dobriyan
2008-05-20 21:17 ` Robert Olsson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m1lk257p5k.fsf@frodo.ebiederm.org \
    --to=ebiederm@xmission$(echo .)com \
    --cc=akpm@linux-foundation$(echo .)org \
    --cc=bugme-daemon@bugzilla$(echo .)kernel.org \
    --cc=den@openvz$(echo .)org \
    --cc=devzero@web$(echo .)de \
    --cc=greearb@candelatech$(echo .)com \
    --cc=kaber@trash$(echo .)net \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=robert.olsson@its$(echo .)uu.se \
    --cc=xemul@openvz$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox