linux-kernel - Re: [PATCH 09/13] auxdisplay: charlcd: Extract character LCD core from misc/panel

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:   Wed, 8 Feb 2017 13:22:13 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Miguel Ojeda Sandonis <miguel.ojeda.sandonis@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Willy Tarreau <willy@...roxy.com>,
        Ksenija Stanojevic <ksenija.stanojevic@...il.com>,
        Arnd Bergmann <arnd@...db.de>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>
Subject: Re: [PATCH 09/13] auxdisplay: charlcd: Extract character LCD core
 from misc/panel

On Mon, Feb 6, 2017 at 3:38 PM, Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> Extract the character LCD core from the Parallel port LCD/Keypad Panel
> driver in the misc subsystem, and convert it into a subdriver in the
> auxdisplay subsystem.  This allows the character LCD core to be used by
> other drivers later.
>
> Compilation is controlled by its own Kconfig symbol CHARLCD, which is to
> be selected by its users, but can be enabled manually for
> compile-testing.
>
> All functions changed their prefix from "lcd_" to "charlcd_", and gained
> a "struct charlcd *" parameter to operate on a specific instance.
> While the driver API thus is ready to support multiple instances, the
> current limitation of a single display (/dev/lcd has a single misc minor
> assigned) is retained.
>
> No functional changes intended.
>
> Signed-off-by: Geert Uytterhoeven <geert@...ux-m68k.org>

> --- /dev/null
> +++ b/drivers/auxdisplay/charlcd.c

> +/* initialize the LCD driver */
> +static int charlcd_init(struct charlcd *lcd)
> +{
> +       struct charlcd_priv *priv = to_priv(lcd);
> +       int ret;
> +
> +       /*
> +        * before this line, we must NOT send anything to the display.
> +        * Since charlcd_init_display() needs to write data, we have to
> +        * enable mark the LCD initialized just before.
> +        */
> +       ret = charlcd_init_display(lcd);

0day reported:

| INFO: trying to register non-static key.
| the code is fine but needs lockdep annotation.
| turning off the locking correctness validator.

However, the code is not fine.
charlcd_init_display() calls charlcd_backlight(), which acquires the
mutex that is only initialized below.

> +       if (ret)
> +               return ret;
> +
> +       if (lcd->ops->backlight) {
> +               mutex_init(&priv->bl_tempo_lock);
> +               INIT_DELAYED_WORK(&priv->bl_work, charlcd_bl_off);
> +       }

Moving the block above before the call to charlcd_init_display() fixes this.
Will do so in v2.

Note that "misc: panel: Abstract temporary backlight handling" doesn't
suffer from this bug.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds