From: Huang Jianan <huangjianan@oppo.com> To: Chengguang Xu <cgxu519@139.com>, linux-unionfs@vger.kernel.org, miklos@szeredi.hu, linux-erofs@lists.ozlabs.org, xiang@kernel.org, chao@kernel.org Cc: guoweichao@oppo.com, yh@oppo.com, zhangshiming@oppo.com, guanyuwei@oppo.com, jnhuang95@gmail.com, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v3] ovl: fix null pointer when filesystem doesn't support direct IO Date: Wed, 22 Sep 2021 16:24:56 +0800 [thread overview] Message-ID: <919e929d-6af7-b729-9fd2-954cd1e52999@oppo.com> (raw) In-Reply-To: <e42a183f-274c-425f-2012-3ff0003e1fcb@139.com> 在 2021/9/22 16:06, Chengguang Xu 写道: > 在 2021/9/22 15:23, Huang Jianan 写道: >> From: Huang Jianan <huangjianan@oppo.com> >> >> At present, overlayfs provides overlayfs inode to users. Overlayfs >> inode provides ovl_aops with noop_direct_IO to avoid open failure >> with O_DIRECT. But some compressed filesystems, such as erofs and >> squashfs, don't support direct_IO. >> >> Users who use f_mapping->a_ops->direct_IO to check O_DIRECT support, >> will read file through this way. This will cause overlayfs to access >> a non-existent direct_IO function and cause panic due to null pointer: > > I just looked around the code more closely, in open_with_fake_path(), > > do_dentry_open() has already checked O_DIRECT open flag and > a_ops->direct_IO of underlying real address_space. > > Am I missing something? > > It seems that loop_update_dio will set lo->use_dio after open file without set O_DIRECT. loop_update_dio will check f_mapping->a_ops->direct_IO but it deal with ovl_aops with noop _direct_IO. So I think we still need a new aops? Thanks, Jianan > Thanks, > > Chengguang > > >> >> Kernel panic - not syncing: CFI failure (target: 0x0) >> CPU: 6 PID: 247 Comm: loop0 >> Call Trace: >> panic+0x188/0x45c >> __cfi_slowpath+0x0/0x254 >> __cfi_slowpath+0x200/0x254 >> generic_file_read_iter+0x14c/0x150 >> vfs_iocb_iter_read+0xac/0x164 >> ovl_read_iter+0x13c/0x2fc >> lo_rw_aio+0x2bc/0x458 >> loop_queue_work+0x4a4/0xbc0 >> kthread_worker_fn+0xf8/0x1d0 >> loop_kthread_worker_fn+0x24/0x38 >> kthread+0x29c/0x310 >> ret_from_fork+0x10/0x30 >> >> The filesystem may only support direct_IO for some file types. For >> example, erofs supports direct_IO for uncompressed files. So return >> -EINVAL when the file doesn't support direct_IO to fix this problem. >> >> Fixes: 5b910bd615ba ("ovl: fix GPF in swapfile_activate of file from >> overlayfs over xfs") >> Signed-off-by: Huang Jianan <huangjianan@oppo.com> >> --- >> change since v2: >> - Return error in ovl_open directly. (Chengguang Xu) >> >> Change since v1: >> - Return error to user rather than fall back to buffered io. >> (Chengguang Xu) >> >> fs/overlayfs/file.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c >> index d081faa55e83..a0c99ea35daf 100644 >> --- a/fs/overlayfs/file.c >> +++ b/fs/overlayfs/file.c >> @@ -157,6 +157,10 @@ static int ovl_open(struct inode *inode, struct >> file *file) >> if (IS_ERR(realfile)) >> return PTR_ERR(realfile); >> + if ((f->f_flags & O_DIRECT) && (!realfile->f_mapping->a_ops || >> + !realfile->f_mapping->a_ops->direct_IO)) >> + return -EINVAL; >> + >> file->private_data = realfile; >> return 0; >
WARNING: multiple messages have this Message-ID (diff)
From: Huang Jianan via Linux-erofs <linux-erofs@lists.ozlabs.org> To: Chengguang Xu <cgxu519@139.com>, linux-unionfs@vger.kernel.org, miklos@szeredi.hu, linux-erofs@lists.ozlabs.org, xiang@kernel.org, chao@kernel.org Cc: zhangshiming@oppo.com, linux-kernel@vger.kernel.org, yh@oppo.com, guanyuwei@oppo.com, linux-fsdevel@vger.kernel.org, guoweichao@oppo.com Subject: Re: [PATCH v3] ovl: fix null pointer when filesystem doesn't support direct IO Date: Wed, 22 Sep 2021 16:24:56 +0800 [thread overview] Message-ID: <919e929d-6af7-b729-9fd2-954cd1e52999@oppo.com> (raw) In-Reply-To: <e42a183f-274c-425f-2012-3ff0003e1fcb@139.com> 在 2021/9/22 16:06, Chengguang Xu 写道: > 在 2021/9/22 15:23, Huang Jianan 写道: >> From: Huang Jianan <huangjianan@oppo.com> >> >> At present, overlayfs provides overlayfs inode to users. Overlayfs >> inode provides ovl_aops with noop_direct_IO to avoid open failure >> with O_DIRECT. But some compressed filesystems, such as erofs and >> squashfs, don't support direct_IO. >> >> Users who use f_mapping->a_ops->direct_IO to check O_DIRECT support, >> will read file through this way. This will cause overlayfs to access >> a non-existent direct_IO function and cause panic due to null pointer: > > I just looked around the code more closely, in open_with_fake_path(), > > do_dentry_open() has already checked O_DIRECT open flag and > a_ops->direct_IO of underlying real address_space. > > Am I missing something? > > It seems that loop_update_dio will set lo->use_dio after open file without set O_DIRECT. loop_update_dio will check f_mapping->a_ops->direct_IO but it deal with ovl_aops with noop _direct_IO. So I think we still need a new aops? Thanks, Jianan > Thanks, > > Chengguang > > >> >> Kernel panic - not syncing: CFI failure (target: 0x0) >> CPU: 6 PID: 247 Comm: loop0 >> Call Trace: >> panic+0x188/0x45c >> __cfi_slowpath+0x0/0x254 >> __cfi_slowpath+0x200/0x254 >> generic_file_read_iter+0x14c/0x150 >> vfs_iocb_iter_read+0xac/0x164 >> ovl_read_iter+0x13c/0x2fc >> lo_rw_aio+0x2bc/0x458 >> loop_queue_work+0x4a4/0xbc0 >> kthread_worker_fn+0xf8/0x1d0 >> loop_kthread_worker_fn+0x24/0x38 >> kthread+0x29c/0x310 >> ret_from_fork+0x10/0x30 >> >> The filesystem may only support direct_IO for some file types. For >> example, erofs supports direct_IO for uncompressed files. So return >> -EINVAL when the file doesn't support direct_IO to fix this problem. >> >> Fixes: 5b910bd615ba ("ovl: fix GPF in swapfile_activate of file from >> overlayfs over xfs") >> Signed-off-by: Huang Jianan <huangjianan@oppo.com> >> --- >> change since v2: >> - Return error in ovl_open directly. (Chengguang Xu) >> >> Change since v1: >> - Return error to user rather than fall back to buffered io. >> (Chengguang Xu) >> >> fs/overlayfs/file.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c >> index d081faa55e83..a0c99ea35daf 100644 >> --- a/fs/overlayfs/file.c >> +++ b/fs/overlayfs/file.c >> @@ -157,6 +157,10 @@ static int ovl_open(struct inode *inode, struct >> file *file) >> if (IS_ERR(realfile)) >> return PTR_ERR(realfile); >> + if ((f->f_flags & O_DIRECT) && (!realfile->f_mapping->a_ops || >> + !realfile->f_mapping->a_ops->direct_IO)) >> + return -EINVAL; >> + >> file->private_data = realfile; >> return 0; >
next prev parent reply other threads:[~2021-09-22 8:25 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-18 12:13 [PATCH] ovl: fix null pointer when filesystem doesn't support direct IO Huang Jianan 2021-09-18 12:13 ` Huang Jianan via Linux-erofs 2021-09-22 1:56 ` Chengguang Xu 2021-09-22 1:56 ` Chengguang Xu 2021-09-22 3:39 ` Huang Jianan 2021-09-22 3:39 ` Huang Jianan via Linux-erofs 2021-09-22 3:47 ` [PATCH v2] " Huang Jianan 2021-09-22 3:47 ` Huang Jianan via Linux-erofs 2021-09-22 5:09 ` Chengguang Xu 2021-09-22 5:09 ` Chengguang Xu 2021-09-22 7:18 ` Huang Jianan 2021-09-22 7:18 ` Huang Jianan via Linux-erofs 2021-09-22 7:23 ` [PATCH v3] " Huang Jianan 2021-09-22 7:23 ` Huang Jianan via Linux-erofs 2021-09-22 8:06 ` Chengguang Xu 2021-09-22 8:06 ` Chengguang Xu 2021-09-22 8:24 ` Huang Jianan [this message] 2021-09-22 8:24 ` Huang Jianan via Linux-erofs 2021-09-22 13:20 ` [PATCH v3] ovl: fix null pointer when filesystemdoesn'tsupportdirect IO Chengguang Xu 2021-09-22 13:20 ` Chengguang Xu 2021-09-22 14:00 ` Miklos Szeredi 2021-09-22 14:00 ` Miklos Szeredi 2021-09-27 9:38 ` Miklos Szeredi 2021-09-27 9:38 ` Miklos Szeredi 2021-09-28 7:01 ` Huang Jianan 2021-09-28 7:01 ` Huang Jianan via Linux-erofs 2021-09-28 7:17 ` Miklos Szeredi 2021-09-28 7:17 ` Miklos Szeredi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=919e929d-6af7-b729-9fd2-954cd1e52999@oppo.com \ --to=huangjianan@oppo.com \ --cc=cgxu519@139.com \ --cc=chao@kernel.org \ --cc=guanyuwei@oppo.com \ --cc=guoweichao@oppo.com \ --cc=jnhuang95@gmail.com \ --cc=linux-erofs@lists.ozlabs.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-unionfs@vger.kernel.org \ --cc=miklos@szeredi.hu \ --cc=xiang@kernel.org \ --cc=yh@oppo.com \ --cc=zhangshiming@oppo.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.