Hello Junxiao,

On 5/3/22 14:29, Chang, Junxiao wrote:

We tested Javier's fix, issue couldn't be reproduced anymore.
https://lore.kernel.org/dri-devel/20220502135014.377945-1-javierm@redhat.com/T/#u (local)

Thanks for testing!

But this fix doesn't cover all FB driver's interface:

	.get_unmapped_area = get_fb_unmapped_area,
	.fsync =	fb_deferred_io_fsync,

file_fb_info(file) NULL checking should be added in these two interface functions(get_fb_unmapped_area and fb_deferred_io_fsync) as well?

Yes, I was about to send a new version adding checks for this but I decided
not to. Because by looking at these file ops, I see get_fb_unmapped_area()
is only used when:

#if defined(HAVE_ARCH_FB_UNMAPPED_AREA) || \
	(defined(CONFIG_FB_PROVIDE_GET_FB_UNMAPPED_AREA) && \
	 !defined(CONFIG_MMU))
	.get_unmapped_area = get_fb_unmapped_area,
#endif

so is not really a common configuration and fb_deferred_io_fsync() is not
defined in the same compilation unit, which means that file_fb_info() would
have to be exported (and probably renamed to fb_file_fb_info or something),
which will make the fix more complex and harder to backport to stable.

The fb_release() handler bug in the other hand is quite easy to trigger,
so I'll just keep this fix with the minimum change.

I plan to fix the other two handlers though in a separate patch.
 -- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat