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> <CAGngYiUTJ0TjdTu=UB9yXRJ_mXG8LPmcuBQBDha+T5RHoRUvcQ@mail.gmail.com>
In-Reply-To: <CAGngYiUTJ0TjdTu=UB9yXRJ_mXG8LPmcuBQBDha+T5RHoRUvcQ@mail.gmail.com>
From:   Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date:   Thu, 7 Feb 2019 14:20:56 -0800
Message-ID: <CAKdAkRS3mxXO9+60Lu7_3gqN9XN1CYQJGM5fcta1pG8qXm9wqw@mail.gmail.com>
Subject: Re: [RFC v1 0/3] Address potential user-after-free on module unload
To:     Sven Van Asbroeck <thesven73@gmail.com>
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>,
        Greg KH <gregkh@linuxfoundation.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Sven,

On Thu, Feb 7, 2019 at 1:49 PM Sven Van Asbroeck <thesven73@gmail.com> wrote:
>
> On Tue, Feb 5, 2019 at 9:57 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > Can a Coccinelle script get written to find module-use of the non-devm
> > work init?
>
> Ok so I hacked together a Coccinelle script to find these
> user-after-free issues,
> related to work left running when the device or module is removed.
>
> As far as I can see, these issues may crash/corrupt the kernel even on
> _device_ remove/unplug. Users don't need root for that...
>
> I got 71 hits. At least one is a false positive.
> 34 out of 71 could benefit from devm_init_work().
...
> ./drivers//input/serio/ps2-gpio.c:412:1-18: missing clean-up of
> INIT_WORK/INIT_DELAYED_WORK initialized here (maybe use devm_)

OK, this seems to be a real issue. We need to make sure we flush the
TX work inside ps2_gpio_close().

> ./drivers//input/keyboard/matrix_keypad.c:512:1-18: missing clean-up
> of INIT_WORK/INIT_DELAYED_WORK initialized here

This is not as simple. The work in question is scheduled from
matrix_keypad_start() and matrix_keypad_stop() uses flush_work() to
make sure currently running instance completes after making sure that
it will not be rescheduled. And matrix_keypad_stop() is guaranteed to
be called by input core when input device is being unregistered if
input device was opened. So in effect we do not actually leak work
past driver remove().

Thanks.

-- 
Dmitry