linux-kernel - Re: [PATCH] RFC: siox: don't create a thread without starting it

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]

Message-ID: <20180625125105.GZ2494@hirez.programming.kicks-ass.net>
Date:   Mon, 25 Jun 2018 14:51:05 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     linux-kernel@...r.kernel.org, Oleg Nesterov <oleg@...hat.com>,
        "Eric W . Biederman" <ebiederm@...ssion.com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Gavin Schenk <g.schenk@...elmann.de>, kernel@...gutronix.de
Subject: Re: [PATCH] RFC: siox: don't create a thread without starting it

On Mon, Jun 25, 2018 at 12:20:56PM +0200, Uwe Kleine-König wrote:
> when I just boot without any other siox-related action. So the kthread (created
> in drivers/siox/siox-core.c:siox_master_register()) is never started.
> 
> While you could argue that there is little reason to not start the
> thread there also is little reason to actually do it.

Well, you really _should_ wake up the thread. That first wakeup really
is part of the whole 'create/setup' kthread pattern.

> peterz in #kernelnewbies said "[...] kernel/kthread.c:kthread() should
> really be using __set_current_state(TASK_IDLE), I suppose". This however
> seems to interfere with problems fixed in a076e4bca2fd ("freezer: fix
> kthread_create vs freezer theoretical race").

I don't think so, that patch has an issue with INTERRUPTIBLE, but IDLE
very much doesn't allow signals like INTERRUPTIBLE does.

> So I wonder where the real problem is and how it can be fixed.

Without the first wakeup, the kthread will not run the provided function
and we can therefore argue the creation is incomplete. I really feel you
should just wake the thing up to land in your own wait-condition-loop.

That said, irrespective of the whole UNINTERRUPTIBLE/IDLE thing, I find
this construct fairly fragile. We rely on not getting any spurious
wakeups without a 'special' state. The only reason this doesn't normally
happen is because it's a new task, but since it is already hashed, it
might well be possible to trick someone into sending a wakeup.