Re: Potential use-after-free in ____call_usermodehelper

Dmitry Vyukov
2013-09-03T20:50:20Z

On Wed, Sep 4, 2013 at 12:27 AM, Dan Aloni <alonid@stratoscale.com> wrote:
> On Tue, Sep 03, 2013 at 05:49:03PM +0400, Dmitry Vyukov wrote:
>> Is anybody reading this? Is it a correct place to post such things?
>> Maybe there is a more appropriate place?
>
> This is the correct place, and people are reading this. However, the
> bug is not an obvious one, and you mentioned that this bug reproduces
> on a tree with significant deviation from vanilla in core kernel code
> (i.e. memory allocations) with potential bugs, and it also relies on a
> gcc feature not used by the kernel yet.
>
> Because of that it went down in priority compared to other kernel bugs
> that are currently being investigated.

Thanks Dan!

I am new here, so I was thinking maybe I am doing something wrong.

> Your work and contribution is appreciated nonetheless, but your bug
> report needs to pertain closer to the work the core kernel hackers
> are doing.

What exactly do you mean?

> Despite that, I did look into your report, and I'll address your analysis:
>
>> >> I've looked at the sources, but I can't say that I fully understand
>> >> them. The report looks valid, though. I see several potential issues
>> >> in the code.
>> >>
>> >> 1. When wait=UMH_WAIT_EXEC and do_execve() fails,
>> >> ____call_usermodehelper() writes sub_info->retval=retval to freed
>> >> memory. This is the use-after-free reported by the tool.
>
> The 'pid = kernel_thread(call_helper, sub_info, CLONE_VFORK | SIGCHLD);'
> call is designed to block until either do_execve() succeeds or the kernel
> thread exits - that happens after the store that triggers the issue,
> so the use-after-free cannot occur in that case.
>
> This is thanks to the CLONE_VFORK flag, and I doubt it is broken.

Aha! I've missed VFORK. It's all like in userspace, right? I will redo
my analysis.

>> >> 2. When wait=UMH_NO_WAIT, __call_usermodehelper() starts child thread
>> >> and instantly frees subprocess_info. The child thread reads
>> >> subprocess_info. Looks like another use-after-free.
>
> Same, kernel_thread() would be blocking because of CLONE_VFORK.
>
>> >> 3. UMH_WAIT_EXEC does not actually wait for exec, it only waits for
>> >> starting the child thread that will do exec. I don't know whether it's
>> >> a problem with the code or with the name.
>
> Same.
>
>> >>
>> >> The kernel version is 3.11-rc4 (last commit:
>> >> b7bc9e7d808ba55729bd263b0210cda36965be32).
>> >>
>> >> Please help to confirm these issues, and advice what to do next with them.
>
> Theory aside, I tried in practice to confirm the suggested issues and was
> unsuccessful, CLONE_VFORK seems to work as expected. Note that
> /sys/kernel/uevent_helper is empty in recent distros, so I had to put a
> non-existing executable path in there, for the relevant code path to run.
>
> Concerning the original reporting, my suggestion is that you take the
> patch below or a similar idea and try to reproduce the assert it adds
> with the *vanilla* kernel. i.e, take the unmodified b7bc9e7d808ba5 version
> (or better - v3.11), and apply it. Also, you must remove the standard
> memory debugging options from the config of the kernel you that build, so
> that kfree would not zero out the memory (I think this is the default
> behavior in non-debug kernel, for performance).
>
> diff --git a/kernel/kmod.c b/kernel/kmod.c
> index 8241906..d10eab6 100644
> --- a/kernel/kmod.c
> +++ b/kernel/kmod.c
> @@ -244,6 +244,7 @@ static int ____call_usermodehelper(void *data)
>
> /* Exec failed? */
> fail:
> + BUG_ON(sub_info->retval == 0x12345678);
> sub_info->retval = retval;
> do_exit(0);
> }
> @@ -259,6 +260,7 @@ static void call_usermodehelper_freeinfo(struct subprocess_info *info)
> {
> if (info->cleanup)
> (*info->cleanup)(info);
> + info->retval = 0x12345678;
> kfree(info);
> }
>
> This should help to prove it or not, and if it does prove it would be
> appealing to more eyes. Please try the same approach with similar future
> issues.

OK, I will try to do it. It may be possible for this "bug", because
it's triggered quite often. It may not be possible for all similar
issues, because I've seen some of them only once.

Btw, is it better to report such things here or file on bugzilla.kernel.org?

Thanks again!