linux-kernel - Re: [PATCH] fbcon: Remove fbcon_has

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6b9747cf-8845-0eb9-878e-f2953665fcec@samsung.com>
Date:   Wed, 22 May 2019 12:04:33 +0200
From:   Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
To:     Daniel Vetter <daniel.vetter@...ll.ch>
Cc:     DRI Development <dri-devel@...ts.freedesktop.org>,
        Intel Graphics Development <intel-gfx@...ts.freedesktop.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Daniel Vetter <daniel.vetter@...el.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Noralf Trønnes <noralf@...nnes.org>,
        Yisheng Xie <ysxie@...mail.com>,
        Konstantin Khorenko <khorenko@...tuozzo.com>,
        Prarit Bhargava <prarit@...hat.com>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH] fbcon: Remove fbcon_has_exited


Hi Daniel,

On 5/21/19 4:23 PM, Daniel Vetter wrote:
> This is unused code since
> 
> commit 6104c37094e729f3d4ce65797002112735d49cd1
> Author: Daniel Vetter <daniel.vetter@...ll.ch>
> Date:   Tue Aug 1 17:32:07 2017 +0200
> 
>     fbcon: Make fbcon a built-time depency for fbdev
> 
> when fbcon was made a compile-time static dependency of fbdev. We
> can't exit fbcon anymore without exiting fbdev first, which only works
> if all fbdev drivers have unloaded already. Hence this is all dead
> code.
> 
> v2: I missed that fbcon_exit is also called from con_deinit stuff, and
> there fbcon_has_exited prevents double-cleanup. But we can fix that
> by properly resetting con2fb_map[] to all -1, which is used everywhere
> else to indicate "no fb_info allocate to this console". With that
> change the double-cleanup (which resulted in a module refcount underflow,
> among other things) is prevented.
> 
> Aside: con2fb_map is a signed char, so don't register more than 128 fb_info
> or hilarity will ensue.
> 
> v3: CI showed me that I still didn't fully understand what's going on
> here. The leaked references in con2fb_map have been used upon
> rebinding the fb console in fbcon_init. It worked because fbdev
> unregistering still cleaned out con2fb_map, and reset it to info_idx.
> If the last fbdev driver unregistered, then it also reset info_idx,
> and unregistered the fbcon driver.
> 
> Imo that's all a bit fragile, so let's keep the con2fb_map reset to
> -1, and in fbcon_init pick info_idx if we're starting fresh. That
> means unbinding and rebinding will cleanse the mapping, but why are
> you doing that if you want to retain the mapping, so should be fine.
> 
> Also, I think info_idx == -1 is impossible in fbcon_init - we
> unregister the fbcon in that case. So catch&warn about that.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@...el.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
> Cc: Daniel Vetter <daniel.vetter@...ll.ch>
> Cc: Hans de Goede <hdegoede@...hat.com>
> Cc: "Noralf Trønnes" <noralf@...nnes.org>
> Cc: Yisheng Xie <ysxie@...mail.com>
> Cc: Konstantin Khorenko <khorenko@...tuozzo.com>
> Cc: Prarit Bhargava <prarit@...hat.com>
> Cc: Kees Cook <keescook@...omium.org>
> ---
>  drivers/video/fbdev/core/fbcon.c | 39 +++++---------------------------
>  1 file changed, 6 insertions(+), 33 deletions(-)
This patch was #09/33 in your patch series, now it is independent change.

Do you want me to apply it now or should I wait for the new version of
the whole series?

[ I looked at all patches in the series and they look fine to me.
  After outstanding issues are fixed I'll be happy to apply them all
  to fbdev-for-next (I can create immutable branch if needed). ]

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics