linux-kernel - Re: [PATCH] usermodehelper: Fix -ENOMEM return logic

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:	Thu, 7 Mar 2013 16:47:36 -0300
From:	Lucas De Marchi <lucas.demarchi@...fusion.mobi>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	linux-kernel@...r.kernel.org, David Howells <dhowells@...hat.com>,
	James Morris <james.l.morris@...cle.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] usermodehelper: Fix -ENOMEM return logic

On Thu, Mar 7, 2013 at 4:37 PM, Oleg Nesterov <oleg@...hat.com> wrote:
> Hi Lucas,
>
> On 03/06, Lucas De Marchi wrote:
>>
>> On Mon, Feb 25, 2013 at 3:08 PM, Oleg Nesterov <oleg@...hat.com> wrote:
>> >
>> > So, I hope you will send v2. I'd suggest to split the fixes. 1/3
>> > should create/export the new helpers, and 2-3 fix should call_modprobe()
>> > and call_usermodehelper_keys(). But this is up to you, I won't insist.
>>
>> I was implementing this today, but looking into call_modprobe(), it is
>> never called with UMH_NO_WAIT.
>
> wait == T means UMH_WAIT_PROC, so we can't simply rely on CLONE_VFORK.
> But probably we can rely on sys_wait4.

yep, I was thinking about relying on sys_wait4.

>
> However,
>
>> @@ -98,12 +93,13 @@ static int call_modprobe(char *module_name, int wait)
>>       argv[3] = module_name;  /* check free_modprobe_argv() */
>>       argv[4] = NULL;
>>
>> -     return call_usermodehelper_fns(modprobe_path, argv, envp,
>> -             wait | UMH_KILLABLE, NULL, free_modprobe_argv, NULL);
>> +     ret = call_usermodehelper(modprobe_path, argv, envp,
>> +                               wait | UMH_KILLABLE);
>> +     kfree(module_name);
>
> Please note UMH_KILLABLE. call_usermodehelper() can be interrupted
> and even UMH_WAIT_EXEC case is not safe. If call_modprobe() is killed
> we can return while the workqueue thread still tries to clone/exec/etc.

Even if it's killed, we would just free the resource we allocated
before. It would not be safe if we allocated in the init function and
freed in the cleanup. Or am I missing something?

thanks
Lucas De Marchi
--
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/