linux-kernel - Re: [PATCH] kmod: Fix race in usermodehelper code

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] [day] [month] [year] [list]

Date:	Wed, 1 Jul 2009 20:24:07 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org, Rusty Russell <rusty@...tcorp.com.au>
Subject: Re: [PATCH] kmod: Fix race in usermodehelper code

On Wed, Jul 01, 2009 at 01:56:31PM -0700, Andrew Morton wrote:
> On Tue, 30 Jun 2009 22:48:03 -0400
> Neil Horman <nhorman@...driver.com> wrote:
> 
> > User Mode Helper: Fix race in UMH_WAIT_EXEC case
> > 
> > The user mode helper code has a race in it.  call_usermodehelper_exec takes an
> > allocated subprocess_info structure, which it passes to a workqueue, and then
> > passes it to a kernel thread which it creates, after which it calls complete to
> > signal to the caller of call_usremodehelper_exec that it can free the
> > subprocess_info struct.  But since we use that structure in the created thread,
> > we can't call complete from __call_usermodehelper, which is where we create the
> > kernel_thread.  We need to call complete from within the kernel thread and then
> > not use subprocess_info afterward in the case of UMH_WAIT_EXEC.  Tested
> > successfully by me.
> > 
> 
> <stares at the code for ten minutes>
> 
> Geeze that's getting complex, isn't it?  The patch looks OK.  I think.
> 
It really is.  I had initially considered solving the problem by eliminating the
completion queue, and substituting a refcounting mechanism, but as I wrote it, I
found that the memory allocation and freeing for subprocess_info was going to
need a bunch of augmentation too.  Talk about convoluted.... I decided a simple
fixing of the bug was probably for the best.

> I wonder why people aren't reporting this.  Perhaps CLONE_VFORK plus
> child-runs-first?  (_does_ the chld run first?  We seem to have changed
> it a couple of times)
> 
I think the child currently does run first, but as you note, its not always been
that way, and it might not be that way in the future, especially if the child
queues to a different cpu for whatever reason.  

Neil

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