diff mbox series

[4/6] fbdev: Validate info->screen_{base,buffer} in fb_ops implementations

Message ID 20230425142846.730-5-tzimmermann@suse.de (mailing list archive)
State Not Applicable
Headers show
Series drm,fbdev: Use fbdev's I/O helpers | expand

Commit Message

Thomas Zimmermann April 25, 2023, 2:28 p.m. UTC
Push the test for info->screen_base from fb_read() and fb_write() into
the implementations of struct fb_ops.{fb_read,fb_write}. In cases where
the driver operates on info->screen_buffer, test this field instead.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/cobalt_lcdfb.c     |  6 ++++++
 drivers/video/fbdev/core/fb_sys_fops.c |  6 ++++++
 drivers/video/fbdev/core/fbmem.c       | 10 ++++++++--
 drivers/video/fbdev/sm712fb.c          |  4 ++--
 4 files changed, 22 insertions(+), 4 deletions(-)

Comments

Javier Martinez Canillas April 25, 2023, 4:39 p.m. UTC | #1
Thomas Zimmermann <tzimmermann@suse.de> writes:

> Push the test for info->screen_base from fb_read() and fb_write() into
> the implementations of struct fb_ops.{fb_read,fb_write}. In cases where
> the driver operates on info->screen_buffer, test this field instead.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Sui Jingfeng April 26, 2023, 10:24 a.m. UTC | #2
Tested-by: Sui Jingfeng <suijingfeng@loongson.cn>

On 2023/4/25 22:28, Thomas Zimmermann wrote:
> Push the test for info->screen_base from fb_read() and fb_write() into
> the implementations of struct fb_ops.{fb_read,fb_write}. In cases where
> the driver operates on info->screen_buffer, test this field instead.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>   drivers/video/fbdev/cobalt_lcdfb.c     |  6 ++++++
>   drivers/video/fbdev/core/fb_sys_fops.c |  6 ++++++
>   drivers/video/fbdev/core/fbmem.c       | 10 ++++++++--
>   drivers/video/fbdev/sm712fb.c          |  4 ++--
>   4 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/fbdev/cobalt_lcdfb.c b/drivers/video/fbdev/cobalt_lcdfb.c
> index 5f8b6324d2e8..26dbd1c78195 100644
> --- a/drivers/video/fbdev/cobalt_lcdfb.c
> +++ b/drivers/video/fbdev/cobalt_lcdfb.c
> @@ -129,6 +129,9 @@ static ssize_t cobalt_lcdfb_read(struct fb_info *info, char __user *buf,
>   	unsigned long pos;
>   	int len, retval = 0;
>   
> +	if (!info->screen_base)
> +		return -ENODEV;
> +
>   	pos = *ppos;
>   	if (pos >= LCD_CHARS_MAX || count == 0)
>   		return 0;
> @@ -175,6 +178,9 @@ static ssize_t cobalt_lcdfb_write(struct fb_info *info, const char __user *buf,
>   	unsigned long pos;
>   	int len, retval = 0;
>   
> +	if (!info->screen_base)
> +		return -ENODEV;
> +
>   	pos = *ppos;
>   	if (pos >= LCD_CHARS_MAX || count == 0)
>   		return 0;
> diff --git a/drivers/video/fbdev/core/fb_sys_fops.c b/drivers/video/fbdev/core/fb_sys_fops.c
> index 7dee5d3c7fb1..0cb0989abda6 100644
> --- a/drivers/video/fbdev/core/fb_sys_fops.c
> +++ b/drivers/video/fbdev/core/fb_sys_fops.c
> @@ -22,6 +22,9 @@ ssize_t fb_sys_read(struct fb_info *info, char __user *buf, size_t count,
>   	unsigned long total_size, c;
>   	ssize_t ret;
>   
> +	if (!info->screen_buffer)
> +		return -ENODEV;
> +
>   	total_size = info->screen_size;
>   
>   	if (total_size == 0)
> @@ -61,6 +64,9 @@ ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
>   	unsigned long total_size, c;
>   	size_t ret;
>   
> +	if (!info->screen_buffer)
> +		return -ENODEV;
> +
>   	total_size = info->screen_size;
>   
>   	if (total_size == 0)
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index abc92d79a295..b993bb97058f 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -768,7 +768,7 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>   	int c, cnt = 0, err = 0;
>   	unsigned long total_size;
>   
> -	if (!info || ! info->screen_base)
> +	if (!info)
>   		return -ENODEV;
>   
>   	if (info->state != FBINFO_STATE_RUNNING)
> @@ -777,6 +777,9 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>   	if (info->fbops->fb_read)
>   		return info->fbops->fb_read(info, buf, count, ppos);
>   
> +	if (!info->screen_base)
> +		return -ENODEV;
> +
>   	total_size = info->screen_size;
>   
>   	if (total_size == 0)
> @@ -833,7 +836,7 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
>   	int c, cnt = 0, err = 0;
>   	unsigned long total_size;
>   
> -	if (!info || !info->screen_base)
> +	if (!info)
>   		return -ENODEV;
>   
>   	if (info->state != FBINFO_STATE_RUNNING)
> @@ -842,6 +845,9 @@ fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
>   	if (info->fbops->fb_write)
>   		return info->fbops->fb_write(info, buf, count, ppos);
>   
> +	if (!info->screen_base)
> +		return -ENODEV;
> +
>   	total_size = info->screen_size;
>   
>   	if (total_size == 0)
> diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
> index 6f852cd756c5..b7ad3c644e13 100644
> --- a/drivers/video/fbdev/sm712fb.c
> +++ b/drivers/video/fbdev/sm712fb.c
> @@ -1028,7 +1028,7 @@ static ssize_t smtcfb_read(struct fb_info *info, char __user *buf,
>   	int c, i, cnt = 0, err = 0;
>   	unsigned long total_size;
>   
> -	if (!info || !info->screen_base)
> +	if (!info->screen_base)
>   		return -ENODEV;
>   
>   	total_size = info->screen_size;
> @@ -1091,7 +1091,7 @@ static ssize_t smtcfb_write(struct fb_info *info, const char __user *buf,
>   	int c, i, cnt = 0, err = 0;
>   	unsigned long total_size;
>   
> -	if (!info || !info->screen_base)
> +	if (!info->screen_base)
>   		return -ENODEV;
>   
>   	total_size = info->screen_size;
Geert Uytterhoeven April 26, 2023, 2:56 p.m. UTC | #3
Hi Thomas,


On Tue, Apr 25, 2023 at 4:28 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Push the test for info->screen_base from fb_read() and fb_write() into
> the implementations of struct fb_ops.{fb_read,fb_write}. In cases where
> the driver operates on info->screen_buffer, test this field instead.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Thanks for your patch!
You forgot to mention why it is a good idea to duplicate this in all
the implementations, instead of doing it once in the core?

>  drivers/video/fbdev/cobalt_lcdfb.c     |  6 ++++++
>  drivers/video/fbdev/core/fb_sys_fops.c |  6 ++++++
>  drivers/video/fbdev/core/fbmem.c       | 10 ++++++++--
>  drivers/video/fbdev/sm712fb.c          |  4 ++--
>  4 files changed, 22 insertions(+), 4 deletions(-)

Aren't there more fbdev drivers to fix, before you can move the checks
in drivers/video/fbdev/core/fbmem.c?

Gr{oetje,eeting}s,

                        Geert
Thomas Zimmermann April 27, 2023, 1:54 p.m. UTC | #4
Hi

Am 26.04.23 um 16:56 schrieb Geert Uytterhoeven:
> Hi Thomas,
> 
> 
> On Tue, Apr 25, 2023 at 4:28 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Push the test for info->screen_base from fb_read() and fb_write() into
>> the implementations of struct fb_ops.{fb_read,fb_write}. In cases where
>> the driver operates on info->screen_buffer, test this field instead.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Thanks for your patch!
> You forgot to mention why it is a good idea to duplicate this in all
> the implementations, instead of doing it once in the core?
> 
>>   drivers/video/fbdev/cobalt_lcdfb.c     |  6 ++++++
>>   drivers/video/fbdev/core/fb_sys_fops.c |  6 ++++++
>>   drivers/video/fbdev/core/fbmem.c       | 10 ++++++++--
>>   drivers/video/fbdev/sm712fb.c          |  4 ++--
>>   4 files changed, 22 insertions(+), 4 deletions(-)
> 
> Aren't there more fbdev drivers to fix, before you can move the checks
> in drivers/video/fbdev/core/fbmem.c?

I've found a few. And I've also found quite a number of drivers that use 
screen_base when they should use screen_buffer instead. I'll fix them as 
well.

Best regards
Thomas

> 
> Gr{oetje,eeting}s,
> 
>                          Geert
>
diff mbox series

Patch

diff --git a/drivers/video/fbdev/cobalt_lcdfb.c b/drivers/video/fbdev/cobalt_lcdfb.c
index 5f8b6324d2e8..26dbd1c78195 100644
--- a/drivers/video/fbdev/cobalt_lcdfb.c
+++ b/drivers/video/fbdev/cobalt_lcdfb.c
@@ -129,6 +129,9 @@  static ssize_t cobalt_lcdfb_read(struct fb_info *info, char __user *buf,
 	unsigned long pos;
 	int len, retval = 0;
 
+	if (!info->screen_base)
+		return -ENODEV;
+
 	pos = *ppos;
 	if (pos >= LCD_CHARS_MAX || count == 0)
 		return 0;
@@ -175,6 +178,9 @@  static ssize_t cobalt_lcdfb_write(struct fb_info *info, const char __user *buf,
 	unsigned long pos;
 	int len, retval = 0;
 
+	if (!info->screen_base)
+		return -ENODEV;
+
 	pos = *ppos;
 	if (pos >= LCD_CHARS_MAX || count == 0)
 		return 0;
diff --git a/drivers/video/fbdev/core/fb_sys_fops.c b/drivers/video/fbdev/core/fb_sys_fops.c
index 7dee5d3c7fb1..0cb0989abda6 100644
--- a/drivers/video/fbdev/core/fb_sys_fops.c
+++ b/drivers/video/fbdev/core/fb_sys_fops.c
@@ -22,6 +22,9 @@  ssize_t fb_sys_read(struct fb_info *info, char __user *buf, size_t count,
 	unsigned long total_size, c;
 	ssize_t ret;
 
+	if (!info->screen_buffer)
+		return -ENODEV;
+
 	total_size = info->screen_size;
 
 	if (total_size == 0)
@@ -61,6 +64,9 @@  ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
 	unsigned long total_size, c;
 	size_t ret;
 
+	if (!info->screen_buffer)
+		return -ENODEV;
+
 	total_size = info->screen_size;
 
 	if (total_size == 0)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index abc92d79a295..b993bb97058f 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -768,7 +768,7 @@  fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	int c, cnt = 0, err = 0;
 	unsigned long total_size;
 
-	if (!info || ! info->screen_base)
+	if (!info)
 		return -ENODEV;
 
 	if (info->state != FBINFO_STATE_RUNNING)
@@ -777,6 +777,9 @@  fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
 	if (info->fbops->fb_read)
 		return info->fbops->fb_read(info, buf, count, ppos);
 
+	if (!info->screen_base)
+		return -ENODEV;
+
 	total_size = info->screen_size;
 
 	if (total_size == 0)
@@ -833,7 +836,7 @@  fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 	int c, cnt = 0, err = 0;
 	unsigned long total_size;
 
-	if (!info || !info->screen_base)
+	if (!info)
 		return -ENODEV;
 
 	if (info->state != FBINFO_STATE_RUNNING)
@@ -842,6 +845,9 @@  fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
 	if (info->fbops->fb_write)
 		return info->fbops->fb_write(info, buf, count, ppos);
 
+	if (!info->screen_base)
+		return -ENODEV;
+
 	total_size = info->screen_size;
 
 	if (total_size == 0)
diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
index 6f852cd756c5..b7ad3c644e13 100644
--- a/drivers/video/fbdev/sm712fb.c
+++ b/drivers/video/fbdev/sm712fb.c
@@ -1028,7 +1028,7 @@  static ssize_t smtcfb_read(struct fb_info *info, char __user *buf,
 	int c, i, cnt = 0, err = 0;
 	unsigned long total_size;
 
-	if (!info || !info->screen_base)
+	if (!info->screen_base)
 		return -ENODEV;
 
 	total_size = info->screen_size;
@@ -1091,7 +1091,7 @@  static ssize_t smtcfb_write(struct fb_info *info, const char __user *buf,
 	int c, i, cnt = 0, err = 0;
 	unsigned long total_size;
 
-	if (!info || !info->screen_base)
+	if (!info->screen_base)
 		return -ENODEV;
 
 	total_size = info->screen_size;