linux-ext4 - re: ext4: add a new spinlock i_raw_lock to protect the ext4's raw inode

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>] [day] [month] [year] [list]

Date:	Fri, 20 Mar 2015 13:22:18 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	tytso@....edu
Cc:	linux-ext4@...r.kernel.org
Subject: re: ext4: add a new spinlock i_raw_lock to protect the ext4's raw
 inode

Hello Theodore Ts'o,

The patch 202ee5df38b3: "ext4: add a new spinlock i_raw_lock to
protect the ext4's raw inode" from Apr 21, 2014, leads to the
following static checker warning:

	fs/ext4/inode.c:4359 ext4_do_update_inode()
	warn: we tested 'err' before and it was 'false'

fs/ext4/inode.c
  4351          ext4_inode_csum_set(inode, raw_inode, ei);
  4352          spin_unlock(&ei->i_raw_lock);
  4353          if (inode->i_sb->s_flags & MS_LAZYTIME)
  4354                  ext4_update_other_inodes_time(inode->i_sb, inode->i_ino,
  4355                                                bh->b_data);
  4356  
  4357          BUFFER_TRACE(bh, "call ext4_handle_dirty_metadata");
  4358          rc = ext4_handle_dirty_metadata(handle, NULL, bh);
  4359          if (!err)
  4360                  err = rc;

That patch shifted things around so now we can just say:

		err = ext4_handle_dirty_metadata(handle, NULL, bh);

But my only concern is that should we check the error code?

  4361          ext4_clear_inode_state(inode, EXT4_STATE_NEW);
  4362          if (set_large_file) {
  4363                  BUFFER_TRACE(EXT4_SB(sb)->s_sbh, "get write access");
  4364                  err = ext4_journal_get_write_access(handle, EXT4_SB(sb)->s_sbh);
                        ^^^
We overwrite it if set_large_file is true.

  4365                  if (err)
  4366                          goto out_brelse;
  4367                  ext4_update_dynamic_rev(sb);
  4368                  EXT4_SET_RO_COMPAT_FEATURE(sb,
  4369                                             EXT4_FEATURE_RO_COMPAT_LARGE_FILE);
  4370                  ext4_handle_sync(handle);
  4371                  err = ext4_handle_dirty_super(handle, sb);
  4372          }
  4373          ext4_update_inode_fsync_trans(handle, inode, need_datasync);
  4374  out_brelse:
  4375          brelse(bh);
  4376          ext4_std_error(inode->i_sb, err);
  4377          return err;
  4378  }

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html