Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH V3] fuse: Fix VM_BUG_ON_PAGE issue while accessing zero ref count page
@ 2020-09-06 17:12 Pradeep P V K
  2020-09-07 12:13 ` Dan Carpenter
  0 siblings, 1 reply; 2+ messages in thread
From: Pradeep P V K @ 2020-09-06 17:12 UTC (permalink / raw)
  To: miklos, linux-fsdevel, willy; +Cc: stummala, sayalil, Pradeep P V K

There is a potential race between fuse_abort_conn() and
fuse_copy_page() as shown below, due to which VM_BUG_ON_PAGE
crash is observed for accessing a free page.

context#1:                      context#2:
fuse_dev_do_read()              fuse_abort_conn()
->fuse_copy_args()               ->end_requests()
 ->fuse_copy_pages()              ->request_end()
    ->fuse_copy_page()               ->fuse_writepage_end)
      ->fuse_ref_page()                ->fuse_writepage_free()
                                          ->__free_page()
					    ->put_page_testzero()

      ->get_page()
      ->VM_BUG_ON_PAGE()

This results in below crash as when ->put_page_testzero() in context#2
decrease the page reference and get_page() in context#1 accessed it
with zero page reference count.

[  174.391095]  (1)[10406:Thread-6]page dumped because:
VM_BUG_ON_PAGE(((unsigned int) page_ref_count(page) + 127u <= 127u))
[  174.391113]  (1)[10406:Thread-6]page allocated via order 0,
migratetype Unmovable, gfp_mask
0x620042(GFP_NOFS|__GFP_HIGHMEM|__GFP_HARDWALL), pid 261, ts
174390946312 ns

[  174.391135]  (1)[10406:Thread-6] prep_new_page+0x13c/0x210
[  174.391148]  (1)[10406:Thread-6] get_page_from_freelist+0x21ac/0x2370
[  174.391161]  (1)[10406:Thread-6] __alloc_pages_nodemask+0x244/0x14a8
[  174.391176]  (1)[10406:Thread-6] fuse_writepages_fill+0x150/0x708
[  174.391190]  (1)[10406:Thread-6] write_cache_pages+0x3d8/0x550
[  174.391202]  (1)[10406:Thread-6] fuse_writepages+0x94/0x130
[  174.391214]  (1)[10406:Thread-6] do_writepages+0x74/0x140
[  174.391228]  (1)[10406:Thread-6] __writeback_single_inode+0x168/0x788
[  174.391239]  (1)[10406:Thread-6] writeback_sb_inodes+0x56c/0xab8
[  174.391251]  (1)[10406:Thread-6] __writeback_inodes_wb+0x94/0x180
[  174.391262]  (1)[10406:Thread-6] wb_writeback+0x318/0x618
[  174.391274]  (1)[10406:Thread-6] wb_workfn+0x468/0x828
[  174.391290]  (1)[10406:Thread-6] process_one_work+0x3d0/0x720
[  174.391302]  (1)[10406:Thread-6] worker_thread+0x234/0x4c0
[  174.391314]  (1)[10406:Thread-6] kthread+0x144/0x158
[  174.391327]  (1)[10406:Thread-6] ret_from_fork+0x10/0x1c
[  174.391363]  (1)[10406:Thread-6]------------[ cut here ]------------
[  174.391371]  (1)[10406:Thread-6]kernel BUG at include/linux/mm.h:980!
[  174.391381]  (1)[10406:Thread-6]Internal error: Oops - BUG: 0 [#1]
...
[  174.486928]  (1)[10406:Thread-6]pc : fuse_copy_page+0x750/0x790
[  174.493029]  (1)[10406:Thread-6]lr : fuse_copy_page+0x750/0x790
[  174.718831]  (1)[10406:Thread-6] fuse_copy_page+0x750/0x790
[  174.718838]  (1)[10406:Thread-6] fuse_copy_args+0xb4/0x1e8
[  174.718843]  (1)[10406:Thread-6] fuse_dev_do_read+0x424/0x888
[  174.718848]  (1)[10406:Thread-6] fuse_dev_splice_read+0x94/0x200
[  174.718856]  (1)[10406:Thread-6] __arm64_sys_splice+0x874/0xb20
[  174.718864]  (1)[10406:Thread-6] el0_svc_common+0xc8/0x240
[  174.718869]  (1)[10406:Thread-6] el0_svc_handler+0x6c/0x88
[  174.718875]  (1)[10406:Thread-6] el0_svc+0x8/0xc
[  174.778853]  (1)[10406:Thread-6]Kernel panic - not syncing: Fatal

Fix this by protecting fuse_ref_page() with the same fc->lock as in
fuse_abort_conn().

Changes since V2:
- Moved the spin lock from fuse_copy_pages() to fuse_ref_page()

Changes since V1:
- Modified the logic as per kernel v5.9-rc1.
- Added Reported by tag.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
---
 fs/fuse/dev.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 02b3c36..58c7512 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -875,7 +875,7 @@ static int fuse_try_move_page(struct fuse_copy_state *cs, struct page **pagep)
 }
 
 static int fuse_ref_page(struct fuse_copy_state *cs, struct page *page,
-			 unsigned offset, unsigned count)
+			 unsigned offset, unsigned count, struct fuse_conn *fc)
 {
 	struct pipe_buffer *buf;
 	int err;
@@ -883,6 +883,7 @@ static int fuse_ref_page(struct fuse_copy_state *cs, struct page *page,
 	if (cs->nr_segs >= cs->pipe->max_usage)
 		return -EIO;
 
+	spin_lock(&fc->lock);
 	err = unlock_request(cs->req);
 	if (err)
 		return err;
@@ -892,6 +893,7 @@ static int fuse_ref_page(struct fuse_copy_state *cs, struct page *page,
 	buf = cs->pipebufs;
 	get_page(page);
 	buf->page = page;
+	spin_unlock(&fc->lock);
 	buf->offset = offset;
 	buf->len = count;
 
@@ -907,7 +909,7 @@ static int fuse_ref_page(struct fuse_copy_state *cs, struct page *page,
  * done atomically
  */
 static int fuse_copy_page(struct fuse_copy_state *cs, struct page **pagep,
-			  unsigned offset, unsigned count, int zeroing)
+			  unsigned offset, unsigned count, int zeroing, struct fuse_conn *fc)
 {
 	int err;
 	struct page *page = *pagep;
@@ -917,7 +919,7 @@ static int fuse_copy_page(struct fuse_copy_state *cs, struct page **pagep,
 
 	while (count) {
 		if (cs->write && cs->pipebufs && page) {
-			return fuse_ref_page(cs, page, offset, count);
+			return fuse_ref_page(cs, page, offset, count, fc);
 		} else if (!cs->len) {
 			if (cs->move_pages && page &&
 			    offset == 0 && count == PAGE_SIZE) {
@@ -945,7 +947,7 @@ static int fuse_copy_page(struct fuse_copy_state *cs, struct page **pagep,
 
 /* Copy pages in the request to/from userspace buffer */
 static int fuse_copy_pages(struct fuse_copy_state *cs, unsigned nbytes,
-			   int zeroing)
+			   int zeroing, struct fuse_conn *fc)
 {
 	unsigned i;
 	struct fuse_req *req = cs->req;
@@ -957,7 +959,7 @@ static int fuse_copy_pages(struct fuse_copy_state *cs, unsigned nbytes,
 		unsigned int offset = ap->descs[i].offset;
 		unsigned int count = min(nbytes, ap->descs[i].length);
 
-		err = fuse_copy_page(cs, &ap->pages[i], offset, count, zeroing);
+		err = fuse_copy_page(cs, &ap->pages[i], offset, count, zeroing, fc);
 		if (err)
 			return err;
 
@@ -983,7 +985,7 @@ static int fuse_copy_one(struct fuse_copy_state *cs, void *val, unsigned size)
 /* Copy request arguments to/from userspace buffer */
 static int fuse_copy_args(struct fuse_copy_state *cs, unsigned numargs,
 			  unsigned argpages, struct fuse_arg *args,
-			  int zeroing)
+			  int zeroing, struct fuse_conn *fc)
 {
 	int err = 0;
 	unsigned i;
@@ -991,7 +993,7 @@ static int fuse_copy_args(struct fuse_copy_state *cs, unsigned numargs,
 	for (i = 0; !err && i < numargs; i++)  {
 		struct fuse_arg *arg = &args[i];
 		if (i == numargs - 1 && argpages)
-			err = fuse_copy_pages(cs, arg->size, zeroing);
+			err = fuse_copy_pages(cs, arg->size, zeroing, fc);
 		else
 			err = fuse_copy_one(cs, arg->value, arg->size);
 	}
@@ -1260,7 +1262,7 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
 	err = fuse_copy_one(cs, &req->in.h, sizeof(req->in.h));
 	if (!err)
 		err = fuse_copy_args(cs, args->in_numargs, args->in_pages,
-				     (struct fuse_arg *) args->in_args, 0);
+				     (struct fuse_arg *) args->in_args, 0, fc);
 	fuse_copy_finish(cs);
 	spin_lock(&fpq->lock);
 	clear_bit(FR_LOCKED, &req->flags);
@@ -1590,7 +1592,7 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
 			goto out_iput;
 
 		this_num = min_t(unsigned, num, PAGE_SIZE - offset);
-		err = fuse_copy_page(cs, &page, offset, this_num, 0);
+		err = fuse_copy_page(cs, &page, offset, this_num, 0, fc);
 		if (!err && offset == 0 &&
 		    (this_num == PAGE_SIZE || file_size == end))
 			SetPageUptodate(page);
@@ -1792,7 +1794,7 @@ static struct fuse_req *request_find(struct fuse_pqueue *fpq, u64 unique)
 }
 
 static int copy_out_args(struct fuse_copy_state *cs, struct fuse_args *args,
-			 unsigned nbytes)
+			 unsigned nbytes, struct fuse_conn *fc)
 {
 	unsigned reqsize = sizeof(struct fuse_out_header);
 
@@ -1809,7 +1811,7 @@ static int copy_out_args(struct fuse_copy_state *cs, struct fuse_args *args,
 		lastarg->size -= diffsize;
 	}
 	return fuse_copy_args(cs, args->out_numargs, args->out_pages,
-			      args->out_args, args->page_zeroing);
+			      args->out_args, args->page_zeroing, fc);
 }
 
 /*
@@ -1894,7 +1896,7 @@ static ssize_t fuse_dev_do_write(struct fuse_dev *fud,
 	if (oh.error)
 		err = nbytes != sizeof(oh) ? -EINVAL : 0;
 	else
-		err = copy_out_args(cs, req->args, nbytes);
+		err = copy_out_args(cs, req->args, nbytes, fc);
 	fuse_copy_finish(cs);
 
 	spin_lock(&fpq->lock);
-- 
1.9.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH V3] fuse: Fix VM_BUG_ON_PAGE issue while accessing zero ref count page
  2020-09-06 17:12 [PATCH V3] fuse: Fix VM_BUG_ON_PAGE issue while accessing zero ref count page Pradeep P V K
@ 2020-09-07 12:13 ` Dan Carpenter
  0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2020-09-07 12:13 UTC (permalink / raw)
  To: kbuild, Pradeep P V K, miklos, linux-fsdevel, willy
  Cc: lkp, kbuild-all, stummala, sayalil, Pradeep P V K

[-- Attachment #1: Type: text/plain, Size: 3601 bytes --]

Hi Pradeep,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on fuse/for-next]
[also build test WARNING on v5.9-rc4 next-20200903]
[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]

url:    https://github.com/0day-ci/linux/commits/Pradeep-P-V-K/fuse-Fix-VM_BUG_ON_PAGE-issue-while-accessing-zero-ref-count-page/20200907-111835
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-next
config: i386-randconfig-m021-20200907 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
fs/fuse/dev.c:904 fuse_ref_page() warn: inconsistent returns 'fc->lock'.

Old smatch warnings:
fs/fuse/dev.c:1223 fuse_dev_do_read() warn: inconsistent returns 'fiq->lock'.

# https://github.com/0day-ci/linux/commit/f99df00de1b2265fba75dceb63724c2a9b0f1e52
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Pradeep-P-V-K/fuse-Fix-VM_BUG_ON_PAGE-issue-while-accessing-zero-ref-count-page/20200907-111835
git checkout f99df00de1b2265fba75dceb63724c2a9b0f1e52
vim +904 fs/fuse/dev.c

c3021629a0d820 Miklos Szeredi     2010-05-25  877  static int fuse_ref_page(struct fuse_copy_state *cs, struct page *page,
f99df00de1b226 Pradeep P V K      2020-09-06  878  			 unsigned offset, unsigned count, struct fuse_conn *fc)
c3021629a0d820 Miklos Szeredi     2010-05-25  879  {
c3021629a0d820 Miklos Szeredi     2010-05-25  880  	struct pipe_buffer *buf;
0d8e84b0432beb Miklos Szeredi     2015-07-01  881  	int err;
c3021629a0d820 Miklos Szeredi     2010-05-25  882  
6718b6f855a0b4 David Howells      2019-10-16  883  	if (cs->nr_segs >= cs->pipe->max_usage)
c3021629a0d820 Miklos Szeredi     2010-05-25  884  		return -EIO;
c3021629a0d820 Miklos Szeredi     2010-05-25  885  
f99df00de1b226 Pradeep P V K      2020-09-06  886  	spin_lock(&fc->lock);
dc00809a53edd1 Miklos Szeredi     2015-07-01  887  	err = unlock_request(cs->req);
0d8e84b0432beb Miklos Szeredi     2015-07-01  888  	if (err)
0d8e84b0432beb Miklos Szeredi     2015-07-01  889  		return err;
                                                                ^^^^^^^^^^
Unlock before returning?

0d8e84b0432beb Miklos Szeredi     2015-07-01  890  
c3021629a0d820 Miklos Szeredi     2010-05-25  891  	fuse_copy_finish(cs);
c3021629a0d820 Miklos Szeredi     2010-05-25  892  
c3021629a0d820 Miklos Szeredi     2010-05-25  893  	buf = cs->pipebufs;
09cbfeaf1a5a67 Kirill A. Shutemov 2016-04-01  894  	get_page(page);
c3021629a0d820 Miklos Szeredi     2010-05-25  895  	buf->page = page;
f99df00de1b226 Pradeep P V K      2020-09-06  896  	spin_unlock(&fc->lock);
c3021629a0d820 Miklos Szeredi     2010-05-25  897  	buf->offset = offset;
c3021629a0d820 Miklos Szeredi     2010-05-25  898  	buf->len = count;
c3021629a0d820 Miklos Szeredi     2010-05-25  899  
c3021629a0d820 Miklos Szeredi     2010-05-25  900  	cs->pipebufs++;
c3021629a0d820 Miklos Szeredi     2010-05-25  901  	cs->nr_segs++;
c3021629a0d820 Miklos Szeredi     2010-05-25  902  	cs->len = 0;
c3021629a0d820 Miklos Szeredi     2010-05-25  903  
c3021629a0d820 Miklos Szeredi     2010-05-25 @904  	return 0;
c3021629a0d820 Miklos Szeredi     2010-05-25  905  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36412 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-09-07 12:15 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-06 17:12 [PATCH V3] fuse: Fix VM_BUG_ON_PAGE issue while accessing zero ref count page Pradeep P V K
2020-09-07 12:13 ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).