Re: [PATCH v2] fbdev: defio: fix the pagelist corruption

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3235 bytes --]

Hi

Am 18.03.22 um 01:50 schrieb Chuansheng Liu:
> 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.
> 
> V2: change "int i" to "unsigned int i" (Geert Uytterhoeven)

Thanks a lot. I added to patch to drm-misc-next.

Best regards
Thomas

> 
> Fixes: 105a940416fc ("fbdev/defio: Early-out if page is already
> enlisted")
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> ---
>   drivers/video/fbdev/core/fb_defio.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> index 98b0f23bf5e2..a1da54016c88 100644
> --- a/drivers/video/fbdev/core/fb_defio.c
> +++ b/drivers/video/fbdev/core/fb_defio.c
> @@ -59,7 +59,6 @@ static vm_fault_t fb_deferred_io_fault(struct vm_fault *vmf)
>   		printk(KERN_ERR "no mapping available\n");
>   
>   	BUG_ON(!page->mapping);
> -	INIT_LIST_HEAD(&page->lru);
>   	page->index = vmf->pgoff;
>   
>   	vmf->page = page;
> @@ -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;
> +	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);
>   

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]