* [PATCH] fbdev: Fix invalid page access after closing deferred I/O devices
@ 2023-01-27 16:58 ` Takashi Iwai
0 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2023-01-27 16:58 UTC (permalink / raw)
To: Helge Deller
Cc: linux-fbdev, dri-devel, linux-kernel, Patrik Jakobsson,
Thomas Zimmermann
When a fbdev with deferred I/O is once opened and closed, the dirty
pages still remain queued in the pageref list, and eventually later
those may be processed in the delayed work. This may lead to a
corruption of pages, hitting an Oops.
This patch makes sure to cancel the delayed work and clean up the
pageref list at closing the device for addressing the bug. A part of
the cleanup code is factored out as a new helper function that is
called from the common fb_release().
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/video/fbdev/core/fb_defio.c | 10 +++++++++-
drivers/video/fbdev/core/fbmem.c | 2 ++
include/linux/fb.h | 1 +
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
index c730253ab85c..583cbcf09446 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -313,7 +313,7 @@ void fb_deferred_io_open(struct fb_info *info,
}
EXPORT_SYMBOL_GPL(fb_deferred_io_open);
-void fb_deferred_io_cleanup(struct fb_info *info)
+void fb_deferred_io_release(struct fb_info *info)
{
struct fb_deferred_io *fbdefio = info->fbdefio;
struct page *page;
@@ -327,6 +327,14 @@ void fb_deferred_io_cleanup(struct fb_info *info)
page = fb_deferred_io_page(info, i);
page->mapping = NULL;
}
+}
+EXPORT_SYMBOL_GPL(fb_deferred_io_release);
+
+void fb_deferred_io_cleanup(struct fb_info *info)
+{
+ struct fb_deferred_io *fbdefio = info->fbdefio;
+
+ fb_deferred_io_release(info);
kvfree(info->pagerefs);
mutex_destroy(&fbdefio->lock);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 3a6c8458eb8d..78c4cb5ee7c9 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1454,6 +1454,8 @@ __releases(&info->lock)
struct fb_info * const info = file->private_data;
lock_fb_info(info);
+ if (info->fbdefio)
+ fb_deferred_io_release(info);
if (info->fbops->fb_release)
info->fbops->fb_release(info,1);
module_put(info->fbops->owner);
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 96b96323e9cb..73eb1f85ea8e 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -662,6 +662,7 @@ extern int fb_deferred_io_init(struct fb_info *info);
extern void fb_deferred_io_open(struct fb_info *info,
struct inode *inode,
struct file *file);
+extern void fb_deferred_io_release(struct fb_info *info);
extern void fb_deferred_io_cleanup(struct fb_info *info);
extern int fb_deferred_io_fsync(struct file *file, loff_t start,
loff_t end, int datasync);
--
2.35.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] fbdev: Fix invalid page access after closing deferred I/O devices
@ 2023-01-27 16:58 ` Takashi Iwai
0 siblings, 0 replies; 8+ messages in thread
From: Takashi Iwai @ 2023-01-27 16:58 UTC (permalink / raw)
To: Helge Deller
Cc: Thomas Zimmermann, linux-fbdev, linux-kernel, dri-devel,
Patrik Jakobsson
When a fbdev with deferred I/O is once opened and closed, the dirty
pages still remain queued in the pageref list, and eventually later
those may be processed in the delayed work. This may lead to a
corruption of pages, hitting an Oops.
This patch makes sure to cancel the delayed work and clean up the
pageref list at closing the device for addressing the bug. A part of
the cleanup code is factored out as a new helper function that is
called from the common fb_release().
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/video/fbdev/core/fb_defio.c | 10 +++++++++-
drivers/video/fbdev/core/fbmem.c | 2 ++
include/linux/fb.h | 1 +
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
index c730253ab85c..583cbcf09446 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -313,7 +313,7 @@ void fb_deferred_io_open(struct fb_info *info,
}
EXPORT_SYMBOL_GPL(fb_deferred_io_open);
-void fb_deferred_io_cleanup(struct fb_info *info)
+void fb_deferred_io_release(struct fb_info *info)
{
struct fb_deferred_io *fbdefio = info->fbdefio;
struct page *page;
@@ -327,6 +327,14 @@ void fb_deferred_io_cleanup(struct fb_info *info)
page = fb_deferred_io_page(info, i);
page->mapping = NULL;
}
+}
+EXPORT_SYMBOL_GPL(fb_deferred_io_release);
+
+void fb_deferred_io_cleanup(struct fb_info *info)
+{
+ struct fb_deferred_io *fbdefio = info->fbdefio;
+
+ fb_deferred_io_release(info);
kvfree(info->pagerefs);
mutex_destroy(&fbdefio->lock);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 3a6c8458eb8d..78c4cb5ee7c9 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1454,6 +1454,8 @@ __releases(&info->lock)
struct fb_info * const info = file->private_data;
lock_fb_info(info);
+ if (info->fbdefio)
+ fb_deferred_io_release(info);
if (info->fbops->fb_release)
info->fbops->fb_release(info,1);
module_put(info->fbops->owner);
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 96b96323e9cb..73eb1f85ea8e 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -662,6 +662,7 @@ extern int fb_deferred_io_init(struct fb_info *info);
extern void fb_deferred_io_open(struct fb_info *info,
struct inode *inode,
struct file *file);
+extern void fb_deferred_io_release(struct fb_info *info);
extern void fb_deferred_io_cleanup(struct fb_info *info);
extern int fb_deferred_io_fsync(struct file *file, loff_t start,
loff_t end, int datasync);
--
2.35.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] fbdev: Fix invalid page access after closing deferred I/O devices
2023-01-27 16:58 ` Takashi Iwai
@ 2023-01-28 21:31 ` Patrik Jakobsson
-1 siblings, 0 replies; 8+ messages in thread
From: Patrik Jakobsson @ 2023-01-28 21:31 UTC (permalink / raw)
To: Takashi Iwai
Cc: Helge Deller, linux-fbdev, dri-devel, linux-kernel, Thomas Zimmermann
On Fri, Jan 27, 2023 at 05:58:34PM +0100, Takashi Iwai wrote:
> When a fbdev with deferred I/O is once opened and closed, the dirty
> pages still remain queued in the pageref list, and eventually later
> those may be processed in the delayed work. This may lead to a
> corruption of pages, hitting an Oops.
>
> This patch makes sure to cancel the delayed work and clean up the
> pageref list at closing the device for addressing the bug. A part of
> the cleanup code is factored out as a new helper function that is
> called from the common fb_release().
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Patrik Jakobsson <pjakobsson@suse.de>
> ---
> drivers/video/fbdev/core/fb_defio.c | 10 +++++++++-
> drivers/video/fbdev/core/fbmem.c | 2 ++
> include/linux/fb.h | 1 +
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> index c730253ab85c..583cbcf09446 100644
> --- a/drivers/video/fbdev/core/fb_defio.c
> +++ b/drivers/video/fbdev/core/fb_defio.c
> @@ -313,7 +313,7 @@ void fb_deferred_io_open(struct fb_info *info,
> }
> EXPORT_SYMBOL_GPL(fb_deferred_io_open);
>
> -void fb_deferred_io_cleanup(struct fb_info *info)
> +void fb_deferred_io_release(struct fb_info *info)
> {
> struct fb_deferred_io *fbdefio = info->fbdefio;
> struct page *page;
> @@ -327,6 +327,14 @@ void fb_deferred_io_cleanup(struct fb_info *info)
> page = fb_deferred_io_page(info, i);
> page->mapping = NULL;
> }
> +}
> +EXPORT_SYMBOL_GPL(fb_deferred_io_release);
> +
> +void fb_deferred_io_cleanup(struct fb_info *info)
> +{
> + struct fb_deferred_io *fbdefio = info->fbdefio;
> +
> + fb_deferred_io_release(info);
>
> kvfree(info->pagerefs);
> mutex_destroy(&fbdefio->lock);
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 3a6c8458eb8d..78c4cb5ee7c9 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1454,6 +1454,8 @@ __releases(&info->lock)
> struct fb_info * const info = file->private_data;
>
> lock_fb_info(info);
> + if (info->fbdefio)
> + fb_deferred_io_release(info);
> if (info->fbops->fb_release)
> info->fbops->fb_release(info,1);
> module_put(info->fbops->owner);
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 96b96323e9cb..73eb1f85ea8e 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -662,6 +662,7 @@ extern int fb_deferred_io_init(struct fb_info *info);
> extern void fb_deferred_io_open(struct fb_info *info,
> struct inode *inode,
> struct file *file);
> +extern void fb_deferred_io_release(struct fb_info *info);
> extern void fb_deferred_io_cleanup(struct fb_info *info);
> extern int fb_deferred_io_fsync(struct file *file, loff_t start,
> loff_t end, int datasync);
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fbdev: Fix invalid page access after closing deferred I/O devices
@ 2023-01-28 21:31 ` Patrik Jakobsson
0 siblings, 0 replies; 8+ messages in thread
From: Patrik Jakobsson @ 2023-01-28 21:31 UTC (permalink / raw)
To: Takashi Iwai
Cc: Thomas Zimmermann, Helge Deller, linux-fbdev, dri-devel, linux-kernel
On Fri, Jan 27, 2023 at 05:58:34PM +0100, Takashi Iwai wrote:
> When a fbdev with deferred I/O is once opened and closed, the dirty
> pages still remain queued in the pageref list, and eventually later
> those may be processed in the delayed work. This may lead to a
> corruption of pages, hitting an Oops.
>
> This patch makes sure to cancel the delayed work and clean up the
> pageref list at closing the device for addressing the bug. A part of
> the cleanup code is factored out as a new helper function that is
> called from the common fb_release().
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Patrik Jakobsson <pjakobsson@suse.de>
> ---
> drivers/video/fbdev/core/fb_defio.c | 10 +++++++++-
> drivers/video/fbdev/core/fbmem.c | 2 ++
> include/linux/fb.h | 1 +
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> index c730253ab85c..583cbcf09446 100644
> --- a/drivers/video/fbdev/core/fb_defio.c
> +++ b/drivers/video/fbdev/core/fb_defio.c
> @@ -313,7 +313,7 @@ void fb_deferred_io_open(struct fb_info *info,
> }
> EXPORT_SYMBOL_GPL(fb_deferred_io_open);
>
> -void fb_deferred_io_cleanup(struct fb_info *info)
> +void fb_deferred_io_release(struct fb_info *info)
> {
> struct fb_deferred_io *fbdefio = info->fbdefio;
> struct page *page;
> @@ -327,6 +327,14 @@ void fb_deferred_io_cleanup(struct fb_info *info)
> page = fb_deferred_io_page(info, i);
> page->mapping = NULL;
> }
> +}
> +EXPORT_SYMBOL_GPL(fb_deferred_io_release);
> +
> +void fb_deferred_io_cleanup(struct fb_info *info)
> +{
> + struct fb_deferred_io *fbdefio = info->fbdefio;
> +
> + fb_deferred_io_release(info);
>
> kvfree(info->pagerefs);
> mutex_destroy(&fbdefio->lock);
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 3a6c8458eb8d..78c4cb5ee7c9 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1454,6 +1454,8 @@ __releases(&info->lock)
> struct fb_info * const info = file->private_data;
>
> lock_fb_info(info);
> + if (info->fbdefio)
> + fb_deferred_io_release(info);
> if (info->fbops->fb_release)
> info->fbops->fb_release(info,1);
> module_put(info->fbops->owner);
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 96b96323e9cb..73eb1f85ea8e 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -662,6 +662,7 @@ extern int fb_deferred_io_init(struct fb_info *info);
> extern void fb_deferred_io_open(struct fb_info *info,
> struct inode *inode,
> struct file *file);
> +extern void fb_deferred_io_release(struct fb_info *info);
> extern void fb_deferred_io_cleanup(struct fb_info *info);
> extern int fb_deferred_io_fsync(struct file *file, loff_t start,
> loff_t end, int datasync);
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fbdev: Fix invalid page access after closing deferred I/O devices
2023-01-27 16:58 ` Takashi Iwai
@ 2023-01-29 1:27 ` kernel test robot
-1 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2023-01-29 1:27 UTC (permalink / raw)
To: Takashi Iwai, Helge Deller
Cc: linux-fbdev, Patrik Jakobsson, dri-devel, linux-kernel,
Thomas Zimmermann, oe-kbuild-all
Hi Takashi,
I love your patch! Yet something to improve:
[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.2-rc5 next-20230127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Takashi-Iwai/fbdev-Fix-invalid-page-access-after-closing-deferred-I-O-devices/20230128-180330
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20230127165834.11387-1-tiwai%40suse.de
patch subject: [PATCH] fbdev: Fix invalid page access after closing deferred I/O devices
config: s390-defconfig (https://download.01.org/0day-ci/archive/20230129/202301290917.puRyNsug-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f28e22b16f34068d07913fa5d4fb2c9683aa8dc4
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Takashi-Iwai/fbdev-Fix-invalid-page-access-after-closing-deferred-I-O-devices/20230128-180330
git checkout f28e22b16f34068d07913fa5d4fb2c9683aa8dc4
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/video/fbdev/core/fbmem.c: In function 'fb_release':
>> drivers/video/fbdev/core/fbmem.c:1456:17: error: 'struct fb_info' has no member named 'fbdefio'
1456 | if (info->fbdefio)
| ^~
vim +1456 drivers/video/fbdev/core/fbmem.c
1447
1448 static int
1449 fb_release(struct inode *inode, struct file *file)
1450 __acquires(&info->lock)
1451 __releases(&info->lock)
1452 {
1453 struct fb_info * const info = file->private_data;
1454
1455 lock_fb_info(info);
> 1456 if (info->fbdefio)
1457 fb_deferred_io_release(info);
1458 if (info->fbops->fb_release)
1459 info->fbops->fb_release(info,1);
1460 module_put(info->fbops->owner);
1461 unlock_fb_info(info);
1462 put_fb_info(info);
1463 return 0;
1464 }
1465
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fbdev: Fix invalid page access after closing deferred I/O devices
@ 2023-01-29 1:27 ` kernel test robot
0 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2023-01-29 1:27 UTC (permalink / raw)
To: Takashi Iwai, Helge Deller
Cc: oe-kbuild-all, linux-fbdev, dri-devel, linux-kernel,
Patrik Jakobsson, Thomas Zimmermann
Hi Takashi,
I love your patch! Yet something to improve:
[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.2-rc5 next-20230127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Takashi-Iwai/fbdev-Fix-invalid-page-access-after-closing-deferred-I-O-devices/20230128-180330
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20230127165834.11387-1-tiwai%40suse.de
patch subject: [PATCH] fbdev: Fix invalid page access after closing deferred I/O devices
config: s390-defconfig (https://download.01.org/0day-ci/archive/20230129/202301290917.puRyNsug-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f28e22b16f34068d07913fa5d4fb2c9683aa8dc4
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Takashi-Iwai/fbdev-Fix-invalid-page-access-after-closing-deferred-I-O-devices/20230128-180330
git checkout f28e22b16f34068d07913fa5d4fb2c9683aa8dc4
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/video/fbdev/core/fbmem.c: In function 'fb_release':
>> drivers/video/fbdev/core/fbmem.c:1456:17: error: 'struct fb_info' has no member named 'fbdefio'
1456 | if (info->fbdefio)
| ^~
vim +1456 drivers/video/fbdev/core/fbmem.c
1447
1448 static int
1449 fb_release(struct inode *inode, struct file *file)
1450 __acquires(&info->lock)
1451 __releases(&info->lock)
1452 {
1453 struct fb_info * const info = file->private_data;
1454
1455 lock_fb_info(info);
> 1456 if (info->fbdefio)
1457 fb_deferred_io_release(info);
1458 if (info->fbops->fb_release)
1459 info->fbops->fb_release(info,1);
1460 module_put(info->fbops->owner);
1461 unlock_fb_info(info);
1462 put_fb_info(info);
1463 return 0;
1464 }
1465
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fbdev: Fix invalid page access after closing deferred I/O devices
2023-01-27 16:58 ` Takashi Iwai
@ 2023-01-29 7:58 ` Patrik Jakobsson
-1 siblings, 0 replies; 8+ messages in thread
From: Patrik Jakobsson @ 2023-01-29 7:58 UTC (permalink / raw)
To: Takashi Iwai
Cc: Helge Deller, Thomas Zimmermann, linux-fbdev, linux-kernel,
dri-devel, Patrik Jakobsson
On Fri, Jan 27, 2023 at 5:58 PM Takashi Iwai <tiwai@suse.de> wrote:
>
> When a fbdev with deferred I/O is once opened and closed, the dirty
> pages still remain queued in the pageref list, and eventually later
> those may be processed in the delayed work. This may lead to a
> corruption of pages, hitting an Oops.
>
> This patch makes sure to cancel the delayed work and clean up the
> pageref list at closing the device for addressing the bug. A part of
> the cleanup code is factored out as a new helper function that is
> called from the common fb_release().
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
For some reason my first review didn't make it to the list. Trying
again with my other email.
As kernel test robot says, we need to check CONFIG_FB_DEFERRED_IO
around access to info->fbdefio.
With that fixed:
Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> ---
> drivers/video/fbdev/core/fb_defio.c | 10 +++++++++-
> drivers/video/fbdev/core/fbmem.c | 2 ++
> include/linux/fb.h | 1 +
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> index c730253ab85c..583cbcf09446 100644
> --- a/drivers/video/fbdev/core/fb_defio.c
> +++ b/drivers/video/fbdev/core/fb_defio.c
> @@ -313,7 +313,7 @@ void fb_deferred_io_open(struct fb_info *info,
> }
> EXPORT_SYMBOL_GPL(fb_deferred_io_open);
>
> -void fb_deferred_io_cleanup(struct fb_info *info)
> +void fb_deferred_io_release(struct fb_info *info)
> {
> struct fb_deferred_io *fbdefio = info->fbdefio;
> struct page *page;
> @@ -327,6 +327,14 @@ void fb_deferred_io_cleanup(struct fb_info *info)
> page = fb_deferred_io_page(info, i);
> page->mapping = NULL;
> }
> +}
> +EXPORT_SYMBOL_GPL(fb_deferred_io_release);
> +
> +void fb_deferred_io_cleanup(struct fb_info *info)
> +{
> + struct fb_deferred_io *fbdefio = info->fbdefio;
> +
> + fb_deferred_io_release(info);
>
> kvfree(info->pagerefs);
> mutex_destroy(&fbdefio->lock);
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 3a6c8458eb8d..78c4cb5ee7c9 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1454,6 +1454,8 @@ __releases(&info->lock)
> struct fb_info * const info = file->private_data;
>
> lock_fb_info(info);
> + if (info->fbdefio)
> + fb_deferred_io_release(info);
> if (info->fbops->fb_release)
> info->fbops->fb_release(info,1);
> module_put(info->fbops->owner);
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 96b96323e9cb..73eb1f85ea8e 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -662,6 +662,7 @@ extern int fb_deferred_io_init(struct fb_info *info);
> extern void fb_deferred_io_open(struct fb_info *info,
> struct inode *inode,
> struct file *file);
> +extern void fb_deferred_io_release(struct fb_info *info);
> extern void fb_deferred_io_cleanup(struct fb_info *info);
> extern int fb_deferred_io_fsync(struct file *file, loff_t start,
> loff_t end, int datasync);
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fbdev: Fix invalid page access after closing deferred I/O devices
@ 2023-01-29 7:58 ` Patrik Jakobsson
0 siblings, 0 replies; 8+ messages in thread
From: Patrik Jakobsson @ 2023-01-29 7:58 UTC (permalink / raw)
To: Takashi Iwai
Cc: linux-fbdev, Helge Deller, Patrik Jakobsson, dri-devel,
linux-kernel, Thomas Zimmermann
On Fri, Jan 27, 2023 at 5:58 PM Takashi Iwai <tiwai@suse.de> wrote:
>
> When a fbdev with deferred I/O is once opened and closed, the dirty
> pages still remain queued in the pageref list, and eventually later
> those may be processed in the delayed work. This may lead to a
> corruption of pages, hitting an Oops.
>
> This patch makes sure to cancel the delayed work and clean up the
> pageref list at closing the device for addressing the bug. A part of
> the cleanup code is factored out as a new helper function that is
> called from the common fb_release().
>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
For some reason my first review didn't make it to the list. Trying
again with my other email.
As kernel test robot says, we need to check CONFIG_FB_DEFERRED_IO
around access to info->fbdefio.
With that fixed:
Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> ---
> drivers/video/fbdev/core/fb_defio.c | 10 +++++++++-
> drivers/video/fbdev/core/fbmem.c | 2 ++
> include/linux/fb.h | 1 +
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> index c730253ab85c..583cbcf09446 100644
> --- a/drivers/video/fbdev/core/fb_defio.c
> +++ b/drivers/video/fbdev/core/fb_defio.c
> @@ -313,7 +313,7 @@ void fb_deferred_io_open(struct fb_info *info,
> }
> EXPORT_SYMBOL_GPL(fb_deferred_io_open);
>
> -void fb_deferred_io_cleanup(struct fb_info *info)
> +void fb_deferred_io_release(struct fb_info *info)
> {
> struct fb_deferred_io *fbdefio = info->fbdefio;
> struct page *page;
> @@ -327,6 +327,14 @@ void fb_deferred_io_cleanup(struct fb_info *info)
> page = fb_deferred_io_page(info, i);
> page->mapping = NULL;
> }
> +}
> +EXPORT_SYMBOL_GPL(fb_deferred_io_release);
> +
> +void fb_deferred_io_cleanup(struct fb_info *info)
> +{
> + struct fb_deferred_io *fbdefio = info->fbdefio;
> +
> + fb_deferred_io_release(info);
>
> kvfree(info->pagerefs);
> mutex_destroy(&fbdefio->lock);
> diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> index 3a6c8458eb8d..78c4cb5ee7c9 100644
> --- a/drivers/video/fbdev/core/fbmem.c
> +++ b/drivers/video/fbdev/core/fbmem.c
> @@ -1454,6 +1454,8 @@ __releases(&info->lock)
> struct fb_info * const info = file->private_data;
>
> lock_fb_info(info);
> + if (info->fbdefio)
> + fb_deferred_io_release(info);
> if (info->fbops->fb_release)
> info->fbops->fb_release(info,1);
> module_put(info->fbops->owner);
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 96b96323e9cb..73eb1f85ea8e 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -662,6 +662,7 @@ extern int fb_deferred_io_init(struct fb_info *info);
> extern void fb_deferred_io_open(struct fb_info *info,
> struct inode *inode,
> struct file *file);
> +extern void fb_deferred_io_release(struct fb_info *info);
> extern void fb_deferred_io_cleanup(struct fb_info *info);
> extern int fb_deferred_io_fsync(struct file *file, loff_t start,
> loff_t end, int datasync);
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-01-30 8:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-27 16:58 [PATCH] fbdev: Fix invalid page access after closing deferred I/O devices Takashi Iwai
2023-01-27 16:58 ` Takashi Iwai
2023-01-28 21:31 ` Patrik Jakobsson
2023-01-28 21:31 ` Patrik Jakobsson
2023-01-29 1:27 ` kernel test robot
2023-01-29 1:27 ` kernel test robot
2023-01-29 7:58 ` Patrik Jakobsson
2023-01-29 7:58 ` Patrik Jakobsson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.