Hi Chuansheng,
On Thu, Mar 17, 2022 at 7:17 AM Chuansheng Liu <chuansheng.liu@xxxxxxxxx> wrote:
> Easily hit the below list corruption:
> ==
> list_add corruption. prev->next should be next (ffffffffc0ceb090), but
> was ffffec604507edc8. (prev=ffffec604507edc8).
> WARNING: CPU: 65 PID: 3959 at lib/list_debug.c:26
> __list_add_valid+0x53/0x80
> CPU: 65 PID: 3959 Comm: fbdev Tainted: G U
> RIP: 0010:__list_add_valid+0x53/0x80
> Call Trace:
> <TASK>
> fb_deferred_io_mkwrite+0xea/0x150
> do_page_mkwrite+0x57/0xc0
> do_wp_page+0x278/0x2f0
> __handle_mm_fault+0xdc2/0x1590
> handle_mm_fault+0xdd/0x2c0
> do_user_addr_fault+0x1d3/0x650
> exc_page_fault+0x77/0x180
> ? asm_exc_page_fault+0x8/0x30
> asm_exc_page_fault+0x1e/0x30
> RIP: 0033:0x7fd98fc8fad1
> ==
>
> Figure out the race happens when one process is adding &page->lru into
> the pagelist tail in fb_deferred_io_mkwrite(), another process is
> re-initializing the same &page->lru in fb_deferred_io_fault(), which is
> not protected by the lock.
>
> This fix is to init all the page lists one time during initialization,
> it not only fixes the list corruption, but also avoids INIT_LIST_HEAD()
> redundantly.
>
> Fixes: 105a940416fc ("fbdev/defio: Early-out if page is already
> enlisted")
> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> Signed-off-by: Chuansheng Liu <chuansheng.liu@xxxxxxxxx>
Thanks for your patch!
> --- a/drivers/video/fbdev/core/fb_defio.c
> +++ b/drivers/video/fbdev/core/fb_defio.c
> @@ -220,6 +219,8 @@ static void fb_deferred_io_work(struct work_struct *work)
> void fb_deferred_io_init(struct fb_info *info)
> {
> struct fb_deferred_io *fbdefio = info->fbdefio;
> + struct page *page;
> + int i;
unsigned int i;
> BUG_ON(!fbdefio);
> mutex_init(&fbdefio->lock);
> @@ -227,6 +228,12 @@ void fb_deferred_io_init(struct fb_info *info)
> INIT_LIST_HEAD(&fbdefio->pagelist);
> if (fbdefio->delay == 0) /* set a default of 1 s */
> fbdefio->delay = HZ;
> +
> + /* initialize all the page lists one time */
> + for (i = 0; i < info->fix.smem_len; i += PAGE_SIZE) {
> + page = fb_deferred_io_page(info, i);
> + INIT_LIST_HEAD(&page->lru);
> + }
> }
> EXPORT_SYMBOL_GPL(fb_deferred_io_init);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
