linux-kernel - Re: [PATCH] mm/cgroup.c add error check

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]

Date:	Tue, 6 May 2008 22:08:08 -0700
From:	"Paul Menage" <menage@...gle.com>
To:	"KOSAKI Motohiro" <kosaki.motohiro@...fujitsu.com>
Cc:	LKML <linux-kernel@...r.kernel.org>, linux-mm <linux-mm@...ck.org>,
	"Li Zefan" <lizf@...fujitsu.com>
Subject: Re: [PATCH] mm/cgroup.c add error check

On Tue, May 6, 2008 at 4:02 AM, KOSAKI Motohiro
<kosaki.motohiro@...fujitsu.com> wrote:
>
>  on heavy workload, call_usermodehelper() may failure
>  because it use kzmalloc(GFP_ATOMIC).
>
>  but userland want receive release notificcation even heavy workload.
>
>  thus, We should retry if -ENOMEM happend.
>
>
>  Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
>  CC: "Paul Menage" <menage@...gle.com>
>  CC: Li Zefan <lizf@...fujitsu.com>
>
>  ---
>   kernel/cgroup.c |   10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
>  Index: b/kernel/cgroup.c
>  ===================================================================
>  --- a/kernel/cgroup.c   2008-04-29 18:00:53.000000000 +0900
>  +++ b/kernel/cgroup.c   2008-05-06 20:28:23.000000000 +0900
>  @@ -3072,6 +3072,8 @@ void __css_put(struct cgroup_subsys_stat
>   */
>   static void cgroup_release_agent(struct work_struct *work)
>   {
>  +       int err;
>  +
>         BUG_ON(work != &release_agent_work);
>         mutex_lock(&cgroup_mutex);
>         spin_lock(&release_list_lock);
>  @@ -3111,7 +3113,13 @@ static void cgroup_release_agent(struct
>                  * since the exec could involve hitting disk and hence
>                  * be a slow process */
>                 mutex_unlock(&cgroup_mutex);
>  -               call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
>  +
>  +retry:
>  +               err = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
>  +               if (err == -ENOMEM) {
>  +                       schedule();
>  +                       goto retry;
>  +               }

I'm not sure that an infinite loop retry is a great idea. Assuming
that call_usermodehelper() is changed to not use GFP_ATOMIC (which
should be safe at least in the case when UMG_WAIT_EXEC is set, since
we can clearly sleep in that case) then I'd be inclined not to check.
If we're so low on memory that we can't fork/exec, then the helper
binary itself could just as easily OOM as soon as we've launched it.
So there's still no guarantee. Userspace should just have to check for
empty cgroups occasionally even if they are using notify_on_release.

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/