linux-kernel - Re: [PATCH] cgroup: Fix root_count when mount fails due to busy subsystem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6599ad830901230859p5453b166tfb384caad210f84b@mail.gmail.com>
Date:	Fri, 23 Jan 2009 08:59:10 -0800
From:	Paul Menage <menage@...gle.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	akpm@...ux-foundation.org, serue@...ibm.com,
	linux-kernel@...r.kernel.org, containers@...ts.osdl.org,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH] cgroup: Fix root_count when mount fails due to busy 
	subsystem

On Fri, Jan 23, 2009 at 2:22 AM, Ingo Molnar <mingo@...e.hu> wrote:
>> --- a/kernel/cgroup.c
>> +++ b/kernel/cgroup.c
>> @@ -1115,8 +1115,10 @@ static void cgroup_kill_sb(struct super_block *sb) {
>>       }
>>       write_unlock(&css_set_lock);
>>
>> -     list_del(&root->root_list);
>> -     root_count--;
>> +     if (!list_empty(&root->root_list)) {
>> +             list_del(&root->root_list);
>> +             root_count--;
>> +     }
>
> That's ugly. It is _much_ cleaner to always keep the link head consistent
> - i.e. initialize it with INIT_LIST_HEAD()

It is initialized with INIT_LIST_HEAD().

> and then remove from it via
> list_del_init().

There's not much point doing list_del_init() rather than list_del()
here since we're about to delete the root.

>
> That way the error path will do the right thing automatically, and there's
> no need for that ugly "if !list_empty" construct either.

The important part here is avoiding decrementing root_count.

So the code could equally be:

if (!list_empty(&root->root_list)) {
  root_count--;
}
list_del(&root->root_list);

but what I have in this patch seems more straightforward. It's
actually how the code used to be before it was removed as a
"redundant" check by a patch that I unfortunately didn't get a chance
to read properly (or Ack) because I was too snowed under with other
work.

Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/