From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild@lists.01.org, Qu Wenruo <wqu@suse.com>,
linux-btrfs@vger.kernel.org
Cc: lkp@intel.com, kbuild-all@lists.01.org
Subject: Re: [PATCH 15/17] btrfs: introduce subpage_eb_mapping for extent buffers
Date: Tue, 8 Sep 2020 17:24:34 +0300 [thread overview]
Message-ID: <20200908142433.GL8299@kadam> (raw)
In-Reply-To: <20200908075230.86856-16-wqu@suse.com>
[-- Attachment #1: Type: text/plain, Size: 12257 bytes --]
Hi Qu,
url: https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-add-read-only-support-for-subpage-sector-size/20200908-155601
base: f4d51dffc6c01a9e94650d95ce0104964f8ae822
config: x86_64-randconfig-m001-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/btrfs/extent_io.c:5516 alloc_extent_buffer() error: uninitialized symbol 'num_pages'.
Old smatch warnings:
fs/btrfs/extent_io.c:6397 try_release_extent_buffer() warn: inconsistent returns 'eb->refs_lock'.
# https://github.com/0day-ci/linux/commit/3ef1cb4eb96ce4dce4dc94e3f06c4dd41879e977
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Qu-Wenruo/btrfs-add-read-only-support-for-subpage-sector-size/20200908-155601
git checkout 3ef1cb4eb96ce4dce4dc94e3f06c4dd41879e977
vim +/subpage_mapping +5511 fs/btrfs/extent_io.c
f28491e0a6c46d Josef Bacik 2013-12-16 5389 struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
ce3e69847e3ec7 David Sterba 2014-06-15 5390 u64 start)
d1310b2e0cd98e Chris Mason 2008-01-24 5391 {
da17066c40472c Jeff Mahoney 2016-06-15 5392 unsigned long len = fs_info->nodesize;
cc5e31a4775d0d David Sterba 2018-03-01 5393 int num_pages;
cc5e31a4775d0d David Sterba 2018-03-01 5394 int i;
09cbfeaf1a5a67 Kirill A. Shutemov 2016-04-01 5395 unsigned long index = start >> PAGE_SHIFT;
d1310b2e0cd98e Chris Mason 2008-01-24 5396 struct extent_buffer *eb;
6af118ce51b52c Chris Mason 2008-07-22 5397 struct extent_buffer *exists = NULL;
d1310b2e0cd98e Chris Mason 2008-01-24 5398 struct page *p;
f28491e0a6c46d Josef Bacik 2013-12-16 5399 struct address_space *mapping = fs_info->btree_inode->i_mapping;
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5400 struct subpage_eb_mapping *subpage_mapping = NULL;
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5401 bool subpage = (fs_info->sectorsize < PAGE_SIZE);
d1310b2e0cd98e Chris Mason 2008-01-24 5402 int uptodate = 1;
19fe0a8b787d7c Miao Xie 2010-10-26 5403 int ret;
d1310b2e0cd98e Chris Mason 2008-01-24 5404
da17066c40472c Jeff Mahoney 2016-06-15 5405 if (!IS_ALIGNED(start, fs_info->sectorsize)) {
c871b0f2fd27e7 Liu Bo 2016-06-06 5406 btrfs_err(fs_info, "bad tree block start %llu", start);
c871b0f2fd27e7 Liu Bo 2016-06-06 5407 return ERR_PTR(-EINVAL);
c871b0f2fd27e7 Liu Bo 2016-06-06 5408 }
039b297b76d816 Qu Wenruo 2020-09-08 5409 if (fs_info->sectorsize < PAGE_SIZE && round_down(start, PAGE_SIZE) !=
039b297b76d816 Qu Wenruo 2020-09-08 5410 round_down(start + len - 1, PAGE_SIZE)) {
039b297b76d816 Qu Wenruo 2020-09-08 5411 btrfs_err(fs_info,
039b297b76d816 Qu Wenruo 2020-09-08 5412 "tree block crosses page boundary, start %llu nodesize %lu",
039b297b76d816 Qu Wenruo 2020-09-08 5413 start, len);
039b297b76d816 Qu Wenruo 2020-09-08 5414 return ERR_PTR(-EINVAL);
039b297b76d816 Qu Wenruo 2020-09-08 5415 }
c871b0f2fd27e7 Liu Bo 2016-06-06 5416
f28491e0a6c46d Josef Bacik 2013-12-16 5417 eb = find_extent_buffer(fs_info, start);
452c75c3d21870 Chandra Seetharaman 2013-10-07 5418 if (eb)
6af118ce51b52c Chris Mason 2008-07-22 5419 return eb;
6af118ce51b52c Chris Mason 2008-07-22 5420
23d79d81b13431 David Sterba 2014-06-15 5421 eb = __alloc_extent_buffer(fs_info, start, len);
2b114d1d33551a Peter 2008-04-01 5422 if (!eb)
c871b0f2fd27e7 Liu Bo 2016-06-06 5423 return ERR_PTR(-ENOMEM);
d1310b2e0cd98e Chris Mason 2008-01-24 5424
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5425 if (subpage) {
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5426 subpage_mapping = kmalloc(sizeof(*subpage_mapping), GFP_NOFS);
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5427 if (!mapping) {
This was probably supposed to be if "subpage_mapping" instead of
"mapping".
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5428 exists = ERR_PTR(-ENOMEM);
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5429 goto free_eb;
The "num_pages" variable is uninitialized on this goto path.
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5430 }
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5431 }
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5432
65ad010488a5cc David Sterba 2018-06-29 5433 num_pages = num_extent_pages(eb);
727011e07cbdf8 Chris Mason 2010-08-06 5434 for (i = 0; i < num_pages; i++, index++) {
d1b5c5671d010d Michal Hocko 2015-08-19 5435 p = find_or_create_page(mapping, index, GFP_NOFS|__GFP_NOFAIL);
c871b0f2fd27e7 Liu Bo 2016-06-06 5436 if (!p) {
c871b0f2fd27e7 Liu Bo 2016-06-06 5437 exists = ERR_PTR(-ENOMEM);
6af118ce51b52c Chris Mason 2008-07-22 5438 goto free_eb;
c871b0f2fd27e7 Liu Bo 2016-06-06 5439 }
4f2de97acee653 Josef Bacik 2012-03-07 5440
4f2de97acee653 Josef Bacik 2012-03-07 5441 spin_lock(&mapping->private_lock);
4f2de97acee653 Josef Bacik 2012-03-07 5442 if (PagePrivate(p)) {
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5443 exists = grab_extent_buffer_from_page(p, start);
b76bd038ff9290 Qu Wenruo 2020-09-08 5444 if (exists && atomic_inc_not_zero(&exists->refs)) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This increment doesn't pair perfectly with the decrement in the error
handling. In other words, we sometimes decrement it when it was never
incremented. This presumably will lead to a use after free.
4f2de97acee653 Josef Bacik 2012-03-07 5445 spin_unlock(&mapping->private_lock);
4f2de97acee653 Josef Bacik 2012-03-07 5446 unlock_page(p);
09cbfeaf1a5a67 Kirill A. Shutemov 2016-04-01 5447 put_page(p);
2457aec63745e2 Mel Gorman 2014-06-04 5448 mark_extent_buffer_accessed(exists, p);
4f2de97acee653 Josef Bacik 2012-03-07 5449 goto free_eb;
4f2de97acee653 Josef Bacik 2012-03-07 5450 }
5ca64f45e92dc5 Omar Sandoval 2015-02-24 5451 exists = NULL;
4f2de97acee653 Josef Bacik 2012-03-07 5452
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5453 if (!subpage) {
4f2de97acee653 Josef Bacik 2012-03-07 5454 /*
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5455 * Do this so attach doesn't complain and we
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5456 * need to drop the ref the old guy had.
4f2de97acee653 Josef Bacik 2012-03-07 5457 */
4f2de97acee653 Josef Bacik 2012-03-07 5458 ClearPagePrivate(p);
0b32f4bbb423f0 Josef Bacik 2012-03-13 5459 WARN_ON(PageDirty(p));
09cbfeaf1a5a67 Kirill A. Shutemov 2016-04-01 5460 put_page(p);
d1310b2e0cd98e Chris Mason 2008-01-24 5461 }
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5462 }
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5463 attach_extent_buffer_page(eb, p, subpage_mapping);
4f2de97acee653 Josef Bacik 2012-03-07 5464 spin_unlock(&mapping->private_lock);
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5465 subpage_mapping = NULL;
0b32f4bbb423f0 Josef Bacik 2012-03-13 5466 WARN_ON(PageDirty(p));
727011e07cbdf8 Chris Mason 2010-08-06 5467 eb->pages[i] = p;
d1310b2e0cd98e Chris Mason 2008-01-24 5468 if (!PageUptodate(p))
d1310b2e0cd98e Chris Mason 2008-01-24 5469 uptodate = 0;
eb14ab8ed24a04 Chris Mason 2011-02-10 5470
eb14ab8ed24a04 Chris Mason 2011-02-10 5471 /*
b16d011e79fb35 Nikolay Borisov 2018-07-04 5472 * We can't unlock the pages just yet since the extent buffer
b16d011e79fb35 Nikolay Borisov 2018-07-04 5473 * hasn't been properly inserted in the radix tree, this
b16d011e79fb35 Nikolay Borisov 2018-07-04 5474 * opens a race with btree_releasepage which can free a page
b16d011e79fb35 Nikolay Borisov 2018-07-04 5475 * while we are still filling in all pages for the buffer and
b16d011e79fb35 Nikolay Borisov 2018-07-04 5476 * we could crash.
eb14ab8ed24a04 Chris Mason 2011-02-10 5477 */
d1310b2e0cd98e Chris Mason 2008-01-24 5478 }
d1310b2e0cd98e Chris Mason 2008-01-24 5479 if (uptodate)
b4ce94de9b4d64 Chris Mason 2009-02-04 5480 set_bit(EXTENT_BUFFER_UPTODATE, &eb->bflags);
115391d2315239 Josef Bacik 2012-03-09 5481 again:
e1860a7724828a David Sterba 2016-05-09 5482 ret = radix_tree_preload(GFP_NOFS);
c871b0f2fd27e7 Liu Bo 2016-06-06 5483 if (ret) {
c871b0f2fd27e7 Liu Bo 2016-06-06 5484 exists = ERR_PTR(ret);
19fe0a8b787d7c Miao Xie 2010-10-26 5485 goto free_eb;
c871b0f2fd27e7 Liu Bo 2016-06-06 5486 }
19fe0a8b787d7c Miao Xie 2010-10-26 5487
f28491e0a6c46d Josef Bacik 2013-12-16 5488 spin_lock(&fs_info->buffer_lock);
f28491e0a6c46d Josef Bacik 2013-12-16 5489 ret = radix_tree_insert(&fs_info->buffer_radix,
d31cf6896006df Qu Wenruo 2020-09-08 5490 start / fs_info->sectorsize, eb);
f28491e0a6c46d Josef Bacik 2013-12-16 5491 spin_unlock(&fs_info->buffer_lock);
19fe0a8b787d7c Miao Xie 2010-10-26 5492 radix_tree_preload_end();
452c75c3d21870 Chandra Seetharaman 2013-10-07 5493 if (ret == -EEXIST) {
f28491e0a6c46d Josef Bacik 2013-12-16 5494 exists = find_extent_buffer(fs_info, start);
452c75c3d21870 Chandra Seetharaman 2013-10-07 5495 if (exists)
6af118ce51b52c Chris Mason 2008-07-22 5496 goto free_eb;
452c75c3d21870 Chandra Seetharaman 2013-10-07 5497 else
452c75c3d21870 Chandra Seetharaman 2013-10-07 5498 goto again;
6af118ce51b52c Chris Mason 2008-07-22 5499 }
6af118ce51b52c Chris Mason 2008-07-22 5500 /* add one reference for the tree */
0b32f4bbb423f0 Josef Bacik 2012-03-13 5501 check_buffer_tree_ref(eb);
34b41acec1ccc0 Josef Bacik 2013-12-13 5502 set_bit(EXTENT_BUFFER_IN_TREE, &eb->bflags);
eb14ab8ed24a04 Chris Mason 2011-02-10 5503
eb14ab8ed24a04 Chris Mason 2011-02-10 5504 /*
b16d011e79fb35 Nikolay Borisov 2018-07-04 5505 * Now it's safe to unlock the pages because any calls to
b16d011e79fb35 Nikolay Borisov 2018-07-04 5506 * btree_releasepage will correctly detect that a page belongs to a
b16d011e79fb35 Nikolay Borisov 2018-07-04 5507 * live buffer and won't free them prematurely.
eb14ab8ed24a04 Chris Mason 2011-02-10 5508 */
28187ae569e8a6 Nikolay Borisov 2018-07-04 5509 for (i = 0; i < num_pages; i++)
28187ae569e8a6 Nikolay Borisov 2018-07-04 5510 unlock_page(eb->pages[i]);
d1310b2e0cd98e Chris Mason 2008-01-24 @5511 return eb;
d1310b2e0cd98e Chris Mason 2008-01-24 5512
6af118ce51b52c Chris Mason 2008-07-22 5513 free_eb:
5ca64f45e92dc5 Omar Sandoval 2015-02-24 5514 WARN_ON(!atomic_dec_and_test(&eb->refs));
3ef1cb4eb96ce4 Qu Wenruo 2020-09-08 5515 kfree(subpage_mapping);
727011e07cbdf8 Chris Mason 2010-08-06 @5516 for (i = 0; i < num_pages; i++) {
727011e07cbdf8 Chris Mason 2010-08-06 5517 if (eb->pages[i])
727011e07cbdf8 Chris Mason 2010-08-06 5518 unlock_page(eb->pages[i]);
727011e07cbdf8 Chris Mason 2010-08-06 5519 }
eb14ab8ed24a04 Chris Mason 2011-02-10 5520
897ca6e9b4fef8 Miao Xie 2010-10-26 5521 btrfs_release_extent_buffer(eb);
6af118ce51b52c Chris Mason 2008-07-22 5522 return exists;
d1310b2e0cd98e Chris Mason 2008-01-24 5523 }
---
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: 38592 bytes --]
next prev parent reply other threads:[~2020-09-08 14:53 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-08 7:52 [PATCH 00/17] btrfs: add read-only support for subpage sector size Qu Wenruo
2020-09-08 7:52 ` [PATCH 01/17] btrfs: extent-io-tests: remove invalid tests Qu Wenruo
2020-09-09 12:26 ` Nikolay Borisov
2020-09-09 13:06 ` Qu Wenruo
2020-09-08 7:52 ` [PATCH 02/17] btrfs: calculate inline extent buffer page size based on page size Qu Wenruo
2020-09-11 9:56 ` Nikolay Borisov
2020-09-11 10:13 ` Qu Wenruo
2020-09-08 7:52 ` [PATCH 03/17] btrfs: remove the open-code to read disk-key Qu Wenruo
2020-09-11 10:07 ` Nikolay Borisov
2020-09-08 7:52 ` [PATCH 04/17] btrfs: make btrfs_fs_info::buffer_radix to take sector size devided values Qu Wenruo
2020-09-11 10:11 ` Nikolay Borisov
2020-09-11 10:15 ` Qu Wenruo
2020-09-08 7:52 ` [PATCH 05/17] btrfs: don't allow tree block to cross page boundary for subpage support Qu Wenruo
2020-09-11 10:26 ` Nikolay Borisov
2020-09-11 11:36 ` Qu Wenruo
2020-09-11 12:08 ` Nikolay Borisov
2020-09-08 7:52 ` [PATCH 06/17] btrfs: handle sectorsize < PAGE_SIZE case for extent buffer accessors Qu Wenruo
2020-09-08 7:52 ` [PATCH 07/17] btrfs: make csum_tree_block() handle sectorsize smaller than page size Qu Wenruo
2020-09-11 11:10 ` Nikolay Borisov
2020-09-08 7:52 ` [PATCH 08/17] btrfs: refactor how we extract extent buffer from page for alloc_extent_buffer() Qu Wenruo
2020-09-11 11:14 ` Nikolay Borisov
2020-09-08 7:52 ` [PATCH 09/17] btrfs: refactor btrfs_release_extent_buffer_pages() Qu Wenruo
2020-09-11 11:17 ` Nikolay Borisov
2020-09-11 11:39 ` Qu Wenruo
2020-09-08 7:52 ` [PATCH 10/17] btrfs: add assert_spin_locked() for attach_extent_buffer_page() Qu Wenruo
2020-09-11 11:22 ` Nikolay Borisov
2020-09-08 7:52 ` [PATCH 11/17] btrfs: extract the extent buffer verification from btree_readpage_end_io_hook() Qu Wenruo
2020-09-11 13:00 ` Nikolay Borisov
2020-09-08 7:52 ` [PATCH 12/17] btrfs: remove the unnecessary parameter @start and @len for check_data_csum() Qu Wenruo
2020-09-11 13:50 ` Nikolay Borisov
2020-09-08 7:52 ` [PATCH 13/17] btrfs: extent_io: only require sector size alignment for page read Qu Wenruo
2020-09-11 13:55 ` Nikolay Borisov
2020-09-15 1:54 ` Qu Wenruo
2020-09-08 7:52 ` [PATCH 14/17] btrfs: make btrfs_readpage_end_io_hook() follow sector size Qu Wenruo
2020-09-09 17:34 ` Goldwyn Rodrigues
2020-09-10 0:05 ` Qu Wenruo
2020-09-10 14:26 ` Goldwyn Rodrigues
2020-09-08 7:52 ` [PATCH 15/17] btrfs: introduce subpage_eb_mapping for extent buffers Qu Wenruo
2020-09-08 10:22 ` kernel test robot
2020-09-08 14:24 ` Dan Carpenter [this message]
2020-09-08 7:52 ` [PATCH 16/17] btrfs: handle extent buffer verification proper for subpage size Qu Wenruo
2020-09-08 7:52 ` [PATCH 17/17] btrfs: allow RO mount of 4K sector size fs on 64K page system Qu Wenruo
2020-09-08 8:03 ` [PATCH 00/17] btrfs: add read-only support for subpage sector size Qu Wenruo
2020-09-11 10:24 ` Qu Wenruo
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=20200908142433.GL8299@kadam \
--to=dan.carpenter@oracle.com \
--cc=kbuild-all@lists.01.org \
--cc=kbuild@lists.01.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=lkp@intel.com \
--cc=wqu@suse.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox