Re: [PATCH v2] fbdev: Use helper to get fb_info in all file operations

Hello Thomas,

On 5/4/22 10:15, Thomas Zimmermann wrote:

Hi

Am 03.05.22 um 22:19 schrieb Javier Martinez Canillas:

quoted

A reference to the framebuffer device struct fb_info is stored in the file
private data, but this reference could no longer be valid and must not be
accessed directly. Instead, the file_fb_info() accessor function must be
used since it does sanity checking to make sure that the fb_info is valid.

This can happen for example if the registered framebuffer device is for a
driver that just uses a framebuffer provided by the system firmware. In
that case, the fbdev core would unregister the framebuffer device when a
real video driver is probed and ask to remove conflicting framebuffers.

Most fbdev file operations already use the helper to get the fb_info but
get_fb_unmapped_area() and fb_deferred_io_fsync() don't. Fix those two.

Since fb_deferred_io_fsync() is not in fbmem.o, the helper has to be
exported. Rename it and add a fb_ prefix to denote that is public now.

Reported-by: Junxiao Chang <redacted>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Thanks.
 

Please see my comment below.

[snip]

quoted

  
+	if (!info)
+		return -ENODEV;
+

This is consistent with other functions, but it's probably not the 
correct errno code. It means that a device is not available for opening.

But the situation here is rather as with close() on a 
disconnected-network file. The call to close() returns EIO in this case. 
Maybe we should consider changing this in a separate patch.

Indeed. Agree that -EIO makes more sense here.
 

Best regards
Thomas

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat