linux-kernel - Re: + softirq-fix-tasklet_kill-and-its-users.patch added to -mm tree

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: <alpine.DEB.2.20.1609220854090.5599@nanos>
Date:   Thu, 22 Sep 2016 09:05:23 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Santosh Shilimkar <santosh.shilimkar@...cle.com>
cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>, ssantosh@...nel.org,
        David Miller <davem@...emloft.net>, giovanni.cabiddu@...el.com,
        gregkh@...uxfoundation.org, herbert@...dor.apana.org.au,
        isdn@...ux-pingi.de, mingo@...e.hu, pebolle@...cali.nl,
        Peter Zijlstra <peterz@...radead.org>,
        salvatore.benedetto@...el.com, tadeusz.struk@...el.com,
        mm-commits@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        linux-next@...r.kernel.org, sergey.senozhatsky@...il.com,
        Ingo Molnar <mingo@...nel.org>
Subject: Re: + softirq-fix-tasklet_kill-and-its-users.patch added to -mm
 tree

B1;2802;0cOn Wed, 21 Sep 2016, Santosh Shilimkar wrote:
> I requested you to include this patch but now am not sure anymore.
> Looks like there are almost 30 more users which are directly
> tweaking 'tasklet_struct' fields and calling other APIs. Hunting them
> and fixing them probably would be an exercise and also those changes
> needs those changed drivers to be tested.
> 
> What do you suggest ? At least this patch needs to be dropped as of now
> till we can have complete coverage for those bad users.

Yes, it needs to be dropped. Stephen, can you please revert it from next?

How to fix this: The only way is to review all tasklet usage sites for
creative abuse and then fix them one by one. This needs to be done anyway
because those are ticking timebombs even without changes in the core
code. I looked at one of the offenders and it's broken today, it's just
protected by the extremly low probablity to hit the wreckage case.

What you can do to coerce the developers/maintainers of offending code into
looking at the mess they created/merged is to implement accessors for the
tasklet struct fields and replace the open coded fiddling with them.

Once that is done, rename the struct fields to something which is absurd
enough to type.  But don't worry, you will find people doing that. I
catched a few brainwrecks who actually used:

 irqdesc->core_internal_state__do_not_mess_with_it

in their code.

Now after having everything converted to accessors, you can add sanity
checks into the accessors and emit WARN_ONCE() when they are used in the
wrong context. That'll make them look and explain why they think that
fiddling in the internals is a good idea.

Thanks,

	tglx