Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190204220952.30761-1-TheSven73@googlemail.com>
 <CAGXu5jJ0g1FUd3mX=RkJCN=BazaWWybAuapvjngpLcWtz3FQGw@mail.gmail.com>
 <CAGngYiUsq55EUKa0rYCeQ62VanhwNUxs-pfkJxUwqd4HO3m-Lg@mail.gmail.com> <20190205184355.GC22198@kroah.com>
In-Reply-To: <20190205184355.GC22198@kroah.com>
From:   Sven Van Asbroeck <thesven73@gmail.com>
Date:   Tue, 5 Feb 2019 14:12:31 -0500
Message-ID: <CAGngYiUMuM_ZxHKVWRihDHZcMncvTwR6Sz1zGNJLrGpyhhT1uQ@mail.gmail.com>
Subject: Re: [RFC v1 0/3] Address potential user-after-free on module unload
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     Kees Cook <keescook@chromium.org>, Tejun Heo <tj@kernel.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Sebastian Reichel <sre@kernel.org>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Feb 5, 2019 at 1:43 PM Greg KH <gregkh@linuxfoundation.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?