linux-ext4 - [bug report] ext4: fast commit recovery path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 27 Oct 2020 11:09:54 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     harshadshirwadkar@...il.com
Cc:     linux-ext4@...r.kernel.org
Subject: [bug report] ext4: fast commit recovery path

Hello Harshad Shirwadkar,

The patch 8016e29f4362: "ext4: fast commit recovery path" from Oct
15, 2020, leads to the following static checker warning:

	fs/ext4/fast_commit.c:1620 ext4_fc_replay_add_range()
	warn: 'path' is an error pointer or valid

fs/ext4/fast_commit.c
  1600          cur = start;
  1601          remaining = len;
  1602          jbd_debug(1, "ADD_RANGE, lblk %d, pblk %lld, len %d, unwritten %d, inode %ld\n",
  1603                    start, start_pblk, len, ext4_ext_is_unwritten(ex),
  1604                    inode->i_ino);
  1605  
  1606          while (remaining > 0) {
  1607                  map.m_lblk = cur;
  1608                  map.m_len = remaining;
  1609                  map.m_pblk = 0;
  1610                  ret = ext4_map_blocks(NULL, inode, &map, 0);
  1611  
  1612                  if (ret < 0) {
  1613                          iput(inode);
  1614                          return 0;
  1615                  }
  1616  
  1617                  if (ret == 0) {
  1618                          /* Range is not mapped */
  1619                          path = ext4_find_extent(inode, cur, NULL, 0);
  1620                          if (!path)
  1621                                  continue;
                                ^^^^^^^^^^^^^^^^
"path" can't be NULL, this should be an IS_ERR() test.  It's sort of
surprising to me that we continue here instead of returning an error.

  1622                          memset(&newex, 0, sizeof(newex));
  1623                          newex.ee_block = cpu_to_le32(cur);
  1624                          ext4_ext_store_pblock(
  1625                                  &newex, start_pblk + cur - start);
  1626                          newex.ee_len = cpu_to_le16(map.m_len);
  1627                          if (ext4_ext_is_unwritten(ex))
  1628                                  ext4_ext_mark_unwritten(&newex);
  1629                          down_write(&EXT4_I(inode)->i_data_sem);
  1630                          ret = ext4_ext_insert_extent(
  1631                                  NULL, inode, &path, &newex, 0);
  1632                          up_write((&EXT4_I(inode)->i_data_sem));
  1633                          ext4_ext_drop_refs(path);
  1634                          kfree(path);
  1635                          if (ret) {
  1636                                  iput(inode);
  1637                                  return 0;
  1638                          }
  1639                          goto next;
  1640                  }
  1641  
  1642                  if (start_pblk + cur - start != map.m_pblk) {

regards,
dan carpenter