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]

Date:   Thu, 22 Sep 2016 09:08:40 -0700
From:   Santosh Shilimkar <santosh.shilimkar@...cle.com>
To:     Thomas Gleixner <tglx@...utronix.de>
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

On 9/22/2016 12:05 AM, Thomas Gleixner wrote:
> 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 Thomas for suggestion and looking into it. Sounds a good plan
to me to tackle this mess. I will give a try and hopefully with help
of those maintainers come up with a series first to fix the
existing bad users. As you said, fixing core will be simple after that.

Regards,
Santosh