Re: [PATCH 15/17] btrfs: introduce subpage_eb_mapping for extent buffers

Dan Carpenter
2020-09-08T14:53:59Z

[-- 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 --]