Re: [RFC PATCH 5/5] kmod: Handle UMH_WAIT_PROC from system unbound workqueue

[Frederic Weisbecker]

On Fri, Jul 10, 2015 at 12:51:17AM +0200, Oleg Nesterov wrote:
> On 07/09, Frederic Weisbecker wrote:
> >
> > The UMH_WAIT_PROC handler runs in its own thread for obsolete reasons.
> > We couldn't launch and then wait for the exec kernel thread completion
> > without blocking other usermodehelper queued jobs since khelper was
> > implemented as a singlthread ordered workqueue.
> >
> > But now we replaced khelper with generic system unbound workqueues which
> > can handle concurrent blocking jobs.
> >
> > So lets run it from the workqueue.
> 
> Probably this is fine, but I am a bit worried...
> 
> WQ_MAX_ACTIVE == 512, this should be enough "in practice". But nothing
> protects us from creative driver(s) which spawns 512 long-living user
> space tasks...
> 
> Note also that userpace can ptrace these task and "block" sys_wait()
> forever.
> 
> I am worried ;)

I am too. And it depends a lot on which workqueue we rely on. If we can't
rely on the existing ones, we'll to create a new one with a high value for
max active and thus potentially a lot of tasks created just for that
usermodehelper thing...

Then again if we can't know, all we can do is stay conservative.
At least we can update the comments to tell about those doubts.

> 
> > CHECK: I'm just worried about the signal handler that gets tweaked
> > and also the call to sys_wait() that might fiddle with internals. The
> > system workqueue must continue to work without surprise for other
> > works.
> 
> Yes. This means that this patch is wrong without disallow_signal()
> at the end.

Yeah. At least that's fixable :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/