linux-kernel - Re: [PATCH 00/33] fbcon notifier begone!

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: <20190525171928.GA13526@ravnborg.org>
Date:   Sat, 25 May 2019 19:19:28 +0200
From:   Sam Ravnborg <sam@...nborg.org>
To:     Daniel Vetter <daniel.vetter@...ll.ch>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Intel Graphics Development <intel-gfx@...ts.freedesktop.org>,
        DRI Development <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH 00/33] fbcon notifier begone!

Hi Daniel.

Good work, nice cleanup all over.

A few comments to a few patches - not something that warrant a
new series to be posted as long as it is fixed before the patches are
applied.


> btw for future plans: I think this is tricky enough (it's old code and all
> that) that we should let this soak for 2-3 kernel releases. I think the
> following would be nice subsequent cleanup/fixes:
> 
> - push the console lock completely from fbmem.c to fbcon.c. I think we're
>   mostly there with prep, but needs to pondering of corner cases.
I wonder - should this code consistently use __acquire() etc so we could
get a little static analysis help for the locking?

I have not played with this for several years and I do not know the
maturity of this today.

> - move fbcon.c from using indices for tracking fb_info (and accessing
>   registered_fbs without proper locking all the time) to real fb_info
>   pointers with the right amount of refcounting. Mostly motivated by the
>   fun I had trying to simplify fbcon_exit().
> 
> - make sure that fbcon call lock/unlock_fb when it calls fbmem.c
>   functions, and sprinkle assert_lockdep_held around in fbmem.c. This
>   needs the console_lock cleanups first.
> 
> But I think that's material for maybe next year or so.
Or maybe after next kernel release.
Could we put this nice plan into todo.rst or similar so we do not have
to hunt it down by asking google?

For the whole series you can add my:
Reviewed-by: Sam Ravnborg <sam@...nborg.org>

Some parts are reviewed as "this looks entirely correct", other parts
I would claim that I actually understood.
And after having spend some hours on this a r-b seems in order.

	Sam