| Message ID | 20220504215151.55082-1-javierm@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | fbdev: Fix use-after-free caused by wrong fb_info cleanup in drivers | expand |
Hi Am 04.05.22 um 23:51 schrieb Javier Martinez Canillas: > Hello, > > This series contains patches suggested by Daniel Vetter to fix a use-after-free > error in the fb_release() function, due a fb_info associated with a fbdev being > freed too early while a user-space process still has the fbdev dev node opened. > > The is cuused by a wrong management of the struct fb_info lifetime in drivers, > but the fbdev core can also be made more resilient about it an leak > > This can easily be reproduced with the simplefb driver doing the following: > > $ cat < /dev/fb0 & > $ echo simple-framebuffer.0 > /sys/bus/platform/drivers/simple-framebuffer/unbind > $ kill %1 > > [ 257.490471] ------------[ cut here ]------------ > ... > [ 257.495125] refcount_t: underflow; use-after-free. > [ 257.495222] WARNING: CPU: 0 PID: 975 at lib/refcount.c:28 refcount_warn_saturate+0xf4/0x144 > ... > [ 257.637482] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 257.644441] pc : refcount_warn_saturate+0xf4/0x144 > [ 257.649226] lr : refcount_warn_saturate+0xf4/0x144 > [ 257.654009] sp : ffff80000a06bbf0 > [ 257.657315] x29: ffff80000a06bbf0 x28: 000000000000000a x27: 000000000000000a > [ 257.664448] x26: 0000000000000000 x25: ffff470b88c6a180 x24: 000000000000000a > [ 257.671581] x23: ffff470b81706480 x22: ffff470b808c2160 x21: ffff470b8922ba20 > [ 257.678713] x20: ffff470b891f5810 x19: ffff470b891f5800 x18: ffffffffffffffff > [ 257.685846] x17: 3a725f7463656a62 x16: ffffbb18c6465fd4 x15: 0720072007200720 > [ 257.692978] x14: 0720072d072d072d x13: 0a2e656572662d72 x12: 657466612d657375 > [ 257.700110] x11: 203b776f6c667265 x10: 646e75203a745f74 x9 : ffffbb18c58f6c90 > [ 257.707242] x8 : 75203b776f6c6672 x7 : 65646e75203a745f x6 : 0000000000000001 > [ 257.714373] x5 : ffff470bff8ec418 x4 : 0000000000000000 x3 : 0000000000000027 > [ 257.721506] x2 : 0000000000000000 x1 : 0000000000000027 x0 : 0000000000000026 > [ 257.728638] Call trace: > [ 257.731075] refcount_warn_saturate+0xf4/0x144 > [ 257.735513] put_fb_info+0x70/0x7c > [ 257.738916] fb_release+0x60/0x74 > [ 257.742225] __fput+0x88/0x240 > [ 257.745276] ____fput+0x1c/0x30 > [ 257.748410] task_work_run+0xc4/0x21c > [ 257.752066] do_exit+0x170/0x370 > [ 257.755288] do_group_exit+0x40/0xb4 > [ 257.758858] get_signal+0x8e0/0x90c > [ 257.762339] do_signal+0x1a0/0x280 > [ 257.765733] do_notify_resume+0xc8/0x390 > [ 257.769650] el0_da+0xe8/0xf0 > [ 257.772613] el0t_64_sync_handler+0xe8/0x130 > [ 257.776877] el0t_64_sync+0x190/0x194 > [ 257.780534] ---[ end trace 0000000000000000 ]--- > > Patch #1 adds a WARN_ON() to framebuffer_release() to prevent the use-after-free > to happen. > > Patch #2 and patch #3 fixes the simplefb and efifb drivers respectively, to > free the resources at the correct place. From a quick look, vesafb seems to be affected as well. Best regards Thomas > > > Daniel Vetter (1): > fbdev: Prevent possible use-after-free in fb_release() > > Javier Martinez Canillas (2): > fbdev/simplefb: Cleanup fb_info in .fb_destroy rather than .remove > fbdev/efifb: Cleanup fb_info in .fb_destroy rather than .remove > > drivers/video/fbdev/core/fbsysfs.c | 4 ++++ > drivers/video/fbdev/efifb.c | 9 ++++++++- > drivers/video/fbdev/simplefb.c | 8 +++++++- > 3 files changed, 19 insertions(+), 2 deletions(-) >
Hello Thomas, On 5/5/22 10:16, Thomas Zimmermann wrote: [snip] >> Patch #1 adds a WARN_ON() to framebuffer_release() to prevent the use-after-free >> to happen. >> >> Patch #2 and patch #3 fixes the simplefb and efifb drivers respectively, to >> free the resources at the correct place. > > From a quick look, vesafb seems to be affected as well. > Right, I wrongly assumed that we only cared about efifb and simplefb but forgot that vesafb is used when setting a VESA mode with vga=foo. I'll add it in a v2. > Best regards > Thomas >