linux-kernel - Re: [RFC v1 0/3] Address potential user-after-free on module unload

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: <CAGngYiUMuM_ZxHKVWRihDHZcMncvTwR6Sz1zGNJLrGpyhhT1uQ@mail.gmail.com>
Date:   Tue, 5 Feb 2019 14:12:31 -0500
From:   Sven Van Asbroeck <thesven73@...il.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Kees Cook <keescook@...omium.org>, Tejun Heo <tj@...nel.org>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Sebastian Reichel <sre@...nel.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>
Subject: Re: [RFC v1 0/3] Address potential user-after-free on module unload

On Tue, Feb 5, 2019 at 1:43 PM Greg KH <gregkh@...uxfoundation.org> wrote:
>
>
> It really should happen when the device is removed (if it is a driver
> that binds to a device.)

Absolutely. That's why I'm advocating adding a devm_init_work(),
which will take care of this automatically.

But it's of course not universally applicable. Not all drivers use devm.

> If this is not a driver, then there should be
> some way to scan that cancel_work_sync() is never called or not, right?

Are you saying the same thing as Kees, that ideally there should be
infrastructure that WARN()s if work isn't cleaned up properly?

I guess for that to work, the code would need to 'know' what resources
the work function is touching. And warn if one of the resources is freed
without cancelling the work.

Also, cancel_work_sync() is only really needed when running the work
on a global or shared workqueue. If it's a private one, then destroy_workqueue()
is good enough to cancel the work.

Sounds like more of a job for static code analysis? Coccinelle?