linux-kernel - [PATCH 0/2] kthread: make struct kthread kmalloc'ed

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]

Date:   Fri, 28 Oct 2016 18:11:06 +0200
From:   Oleg Nesterov <oleg@...hat.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Andy Lutomirski <luto@...capital.net>,
        Roman Pen <roman.penyaev@...fitbricks.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, Tejun Heo <tj@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: [PATCH 0/2] kthread: make struct kthread kmalloc'ed

Sorry for delay, I was distracted...

On 10/26, Thomas Gleixner wrote:
>
> On Wed, 26 Oct 2016, Oleg Nesterov wrote:
> > On 10/26, Thomas Gleixner wrote:
> > >
> > > On Wed, 26 Oct 2016, Oleg Nesterov wrote:
> > > > +static inline void set_kthread_struct(void *kthread)
> > > > +{
> > > > +	/*
> > > > +	 * We abuse ->set_child_tid to avoid the new member and because it
> > > > +	 * can't be wrongly copied by copy_process(). We also rely on fact
> > > > +	 * that the caller can't exec, so PF_KTHREAD can't be cleared.
> > > > +	 */
> > > > +	current->set_child_tid = (__force void __user *)kthread;
> > >
> > > Can we pretty please avoid this type casting? We only have 5 places using
> > > set_child_tid. So we can really make it a proper union
> >
> > Yes, I thought about anonymous union too, the only problem is that
> > it will need more comments ;)
>
> Be careful with anonymous unions. There are a few pitfalls with older
> compilers. That's why I said make it a proper union and fixup the 5 usage
> sites.

Ah. Then I'd prefer to do this later or in a separate change, unless you
feel strongly. I certainly do not want to update other users at least right
now.

Yes, these 2 type casts do not look nice, but they are hidden in the trivial
helpers. And, for example, if something goes wrong we can trivially change
this code to use, say, sas_ss_sp. Just we need to update the comments to
explain why it is safe too.

Finally. I still hope we will kill struct kthread (I mean, unbloat it and
embed into task_struct), and this means that the proper union should touch
more members. Say, sas_* and/or vfork_done+set/clear_child_tid. I'd like
to do this only once if possible.

I'll try to kill to_live_kthread() tomorrow, didn't have time to do this
today.

Oleg.