Re: [PATCH] fbmem: prevent potential use-after-free issues with console_lock()

[Hang Zhang]

On Fri, Jan 6, 2023 at 4:19 PM Daniel Vetter <daniel@xxxxxxxx> wrote:
>
> On Fri, Jan 06, 2023 at 03:25:14PM -0500, Hang Zhang wrote:
> > On Fri, Jan 6, 2023 at 3:05 PM Daniel Vetter <daniel@xxxxxxxx> wrote:
> > > On Fri, Jan 06, 2023 at 02:58:27PM -0500, Hang Zhang wrote:
> > > > On Fri, Jan 6, 2023 at 1:59 PM Daniel Vetter <daniel@xxxxxxxx> wrote:
> > > > BTW, if this is worthed a fix and the performance of console_lock() is a
> > > > major concern, then I think there may be alternative solutions like adding
> > > > a lock_fb_info() to the free call chain - if that's better in performance,
> > > > or maybe selectively protect the matroxfb ioctl but not vblank ioctl as you
> > > > mentioned.
> > >
> > > Please start out with explaining what kind of bug your checker is seeing,
> > > and why. Not how you're trying to fix it. Because I'm pretty sure there
> > > isn't a bug, but since I've already spent a pile of time looking at this,
> > > I want to make sure.
> >
> > We are sorry for the inconvenience caused, we'll follow these practices and
> > guidelines in the future. Thank you!
>
> Once more: Please explain what you're static checker is seeing. I want to
> understanding this, and I'm hoping at least someone involved in this
> static checker can explain what it thinks is going on.
>
> Thanks, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Thank you for your interest, Daniel. The checker tries first to find
the free and
use sites of a certain object (in this case "fb_info"), then reason
about whether
the use can actually happen after the free (e.g., taking into account
factors like
state set/check, locks, etc.), if so, it will flag a potential
use-after-free. As a static
checker, is doesn't execute a program or generate a PoC. We then manually
review each flagged issue by inspecting all related code. In this
case, the checker
(and us) are unaware of the lifetime management logic, which may cause
problems.

Best,
Hang