Re: [PATCH] fbmem: prevent potential use-after-free issues with console_lock()
From: Hang Zhang <hidden>
Date: 2023-01-06 23:20:05
Also in:
dri-devel, lkml
On Fri, Jan 6, 2023 at 5:46 PM Daniel Vetter [off-list ref] wrote:
On Fri, Jan 06, 2023 at 05:12:57PM -0500, Hang Zhang wrote:quoted
On Fri, Jan 6, 2023 at 4:19 PM Daniel Vetter [off-list ref] wrote:quoted
On Fri, Jan 06, 2023 at 03:25:14PM -0500, Hang Zhang wrote:quoted
On Fri, Jan 6, 2023 at 3:05 PM Daniel Vetter [off-list ref] wrote:quoted
On Fri, Jan 06, 2023 at 02:58:27PM -0500, Hang Zhang wrote:quoted
On Fri, Jan 6, 2023 at 1:59 PM Daniel Vetter [off-list ref] 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.chThank 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.Lifetime management is and absolute basic part in the linux kernel. So if your checker flags every free which isn't protected by a lock, then you'll creating endless amounts of false positives. Is this really what you're doing? I'm still very confused ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Hi, Daniel. Lock is only one of many factors we consider in the checker, so the actual workflow is certainly more complicated than "mark every free w/o lock". E.g., we also need to consider the data flow between use and free, the state check, etc. But as you know, it is very challenging for a static checker to automatically and accurately reason about everything in the code (automatic lifetime management analysis can also be tricky for a static analyzer), that's why we perform a manual review afterward. We will continue working on the checker to reduce its false alarms and submit higher-quality reports to the community following your guidelines. Thank you so much for your time! Best, Hang