Re: [PATCH 08/10] Define get_sb_ref()

[Cedric Le Goater <clg-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>]

>> +static int compare_sb_single_ns(struct super_block *sb, void *data)
>> +{
>> +	return sb->s_fs_info == data;
>> +}
>> +
>> +static int set_sb_single_ns(struct super_block *sb, void *data)
>> +{
>> +	struct mq_namespace *mq_ns = (struct mq_namespace *) data;
>> +	int error;
>> +
>> +	sb->s_fs_info = get_mq_ns(mq_ns);
>> +	error = set_anon_super(sb, NULL);
>> +	if (error)
>> +		put_mq_ns(mq_ns);
> 
> Hmm, how come fs/proc/root.c doesn't do a put_pid_ns if set_anon_super()
> failed?  Does it need to do that, or is there some part of the error
> path cleanup at a higher level that would cause that to happen anyway
> (i.e. kill_sb ends up being called anyway)?

the kill_sb ops is only called on an active super_block which can not 
happen if sget() fails. This means that proc_set_super() fails to decrement 
the refcount on the newly created pid namespace if set_anon_super() fails.
That was spotted by Nadia on the mq namespace.

I'll add the following patch to the -lxc patchset to see how it behaves 
under test.

Thanks,

C.

commit f5ab821d6e4ca95bd19d79f7e5ba58a6fb63f6b0
Author: Cedric Le Goater <clg-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>
Date:   Wed Oct 1 14:34:57 2008 +0200

    proc_set_super() fails to decrement the refcount on the pid namespace
    if set_anon_super() fails.
    
    Signed-off-by: Cedric Le Goater <clg-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>

diff --git a/fs/proc/root.c b/fs/proc/root.c
index 9511753..55227d4 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -30,10 +30,14 @@ static int proc_test_super(struct super_block *sb, void *data)
 static int proc_set_super(struct super_block *sb, void *data)
 {
        struct pid_namespace *ns;
+       int error;
 
        ns = (struct pid_namespace *)data;
        sb->s_fs_info = get_pid_ns(ns);
-       return set_anon_super(sb, NULL);
+       error = set_anon_super(sb, NULL);
+       if (error)
+               put_pid_ns(ns);
+       return error;
 }
 
 static int proc_get_sb(struct file_system_type *fs_type,