Re: Potential use-after-free in ____call_usermodehelper

Lucas De Marchi
2013-09-03T21:27:26Z

On Tue, Sep 3, 2013 at 5:49 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> 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.

Another thing is.... try to put in CC who touched this function
lately. I remember touching it some time ago and also Oleg (adding
him).

>
>
>> 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.

yes... the VFORK here is what you were missing.

>
>
>>> >> 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.

Agreed

Lucas De Marchi