| Message ID | 20240911142952.833223-1-kxwang23@m.fudan.edu.cn (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | fbdev: pxafb: Fix use after free vulnerability in pxafb_driver Driver Due to Race Condition | expand |
On 9/11/24 16:29, Kaixin Wang wrote: > In the pxafb_probe function, it calls the pxafb_init_fbinfo function, > after which &fbi->task is associated with pxafb_task. Moreover, > within this pxafb_init_fbinfo function, the pxafb_blank function > within the &pxafb_ops struct is capable of scheduling work. > > If we remove the module which will call pxafb_remove to make cleanup, > it will call unregister_framebuffer function which can call > do_unregister_framebuffer to free fbi->fb through > put_fb_info(fb_info), while the work mentioned above will be used. > The sequence of operations that may lead to a UAF bug is as follows: > > CPU0 CPU1 > > | pxafb_task > pxafb_remove | > unregister_framebuffer(info) | > do_unregister_framebuffer(fb_info) | > put_fb_info(fb_info) | > // free fbi->fb | set_ctrlr_state(fbi, state) > | __pxafb_lcd_power(fbi, 0) > | fbi->lcd_power(on, &fbi->fb.var) > | //use fbi->fb > > Fix it by ensuring that the work is canceled before proceeding > with the cleanup in pxafb_remove. > > Note that only root user can remove the driver at runtime. > > Signed-off-by: Kaixin Wang <kxwang23@m.fudan.edu.cn> > --- > drivers/video/fbdev/pxafb.c | 1 + > 1 file changed, 1 insertion(+) I've added the patch to the fbdev git tree, but changed the title to: "fbdev: pxafb: Fix possible use after free in pxafb_task()" Thanks! Helge
At 2024-09-12 01:38:26, "Helge Deller" <deller@gmx.de> wrote: >On 9/11/24 16:29, Kaixin Wang wrote: >> In the pxafb_probe function, it calls the pxafb_init_fbinfo function, >> after which &fbi->task is associated with pxafb_task. Moreover, >> within this pxafb_init_fbinfo function, the pxafb_blank function >> within the &pxafb_ops struct is capable of scheduling work. >> >> If we remove the module which will call pxafb_remove to make cleanup, >> it will call unregister_framebuffer function which can call >> do_unregister_framebuffer to free fbi->fb through >> put_fb_info(fb_info), while the work mentioned above will be used. >> The sequence of operations that may lead to a UAF bug is as follows: >> >> CPU0 CPU1 >> >> | pxafb_task >> pxafb_remove | >> unregister_framebuffer(info) | >> do_unregister_framebuffer(fb_info) | >> put_fb_info(fb_info) | >> // free fbi->fb | set_ctrlr_state(fbi, state) >> | __pxafb_lcd_power(fbi, 0) >> | fbi->lcd_power(on, &fbi->fb.var) >> | //use fbi->fb >> >> Fix it by ensuring that the work is canceled before proceeding >> with the cleanup in pxafb_remove. >> >> Note that only root user can remove the driver at runtime. >> >> Signed-off-by: Kaixin Wang <kxwang23@m.fudan.edu.cn> >> --- >> drivers/video/fbdev/pxafb.c | 1 + >> 1 file changed, 1 insertion(+) > >I've added the patch to the fbdev git tree, but changed the title to: >"fbdev: pxafb: Fix possible use after free in pxafb_task()" > >Thanks! >Helge > > Thanks for the review! I appreciate the adjustment to the patch title to make it more precise. Best regards, Kaixin Wang
diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c index 2ef56fa28aff..5ce02495cda6 100644 --- a/drivers/video/fbdev/pxafb.c +++ b/drivers/video/fbdev/pxafb.c @@ -2403,6 +2403,7 @@ static void pxafb_remove(struct platform_device *dev) info = &fbi->fb; pxafb_overlay_exit(fbi); + cancel_work_sync(&fbi->task); unregister_framebuffer(info); pxafb_disable_controller(fbi);
In the pxafb_probe function, it calls the pxafb_init_fbinfo function, after which &fbi->task is associated with pxafb_task. Moreover, within this pxafb_init_fbinfo function, the pxafb_blank function within the &pxafb_ops struct is capable of scheduling work. If we remove the module which will call pxafb_remove to make cleanup, it will call unregister_framebuffer function which can call do_unregister_framebuffer to free fbi->fb through put_fb_info(fb_info), while the work mentioned above will be used. The sequence of operations that may lead to a UAF bug is as follows: CPU0 CPU1 | pxafb_task pxafb_remove | unregister_framebuffer(info) | do_unregister_framebuffer(fb_info) | put_fb_info(fb_info) | // free fbi->fb | set_ctrlr_state(fbi, state) | __pxafb_lcd_power(fbi, 0) | fbi->lcd_power(on, &fbi->fb.var) | //use fbi->fb Fix it by ensuring that the work is canceled before proceeding with the cleanup in pxafb_remove. Note that only root user can remove the driver at runtime. Signed-off-by: Kaixin Wang <kxwang23@m.fudan.edu.cn> --- drivers/video/fbdev/pxafb.c | 1 + 1 file changed, 1 insertion(+)