| 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
| ||
|
Message-ID: <aDCbWc9dEzAaQX1J@stanley.mountain>
Date: Fri, 23 May 2025 18:59:21 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Harshad Shirwadkar <harshadshirwadkar@...il.com>
Cc: linux-ext4@...r.kernel.org
Subject: [bug report] ext4: hold s_fc_lock while during fast commit
Hello Harshad Shirwadkar,
Commit 6593714d67ba ("ext4: hold s_fc_lock while during fast commit")
from May 8, 2025 (linux-next), leads to the following Smatch static
checker warning:
fs/ext4/fast_commit.c:1159 ext4_fc_perform_commit()
warn: double unlock '&sbi->s_fc_lock' (orig line 1109)
fs/ext4/fast_commit.c
1039 static int ext4_fc_perform_commit(journal_t *journal)
1040 {
1041 struct super_block *sb = journal->j_private;
1042 struct ext4_sb_info *sbi = EXT4_SB(sb);
1043 struct ext4_inode_info *iter;
1044 struct ext4_fc_head head;
1045 struct inode *inode;
1046 struct blk_plug plug;
1047 int ret = 0;
1048 u32 crc = 0;
1049
1050 /*
1051 * Step 1: Mark all inodes on s_fc_q[MAIN] with
1052 * EXT4_STATE_FC_FLUSHING_DATA. This prevents these inodes from being
1053 * freed until the data flush is over.
1054 */
1055 mutex_lock(&sbi->s_fc_lock);
1056 list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
1057 ext4_set_inode_state(&iter->vfs_inode,
1058 EXT4_STATE_FC_FLUSHING_DATA);
1059 }
1060 mutex_unlock(&sbi->s_fc_lock);
1061
1062 /* Step 2: Flush data for all the eligible inodes. */
1063 ret = ext4_fc_flush_data(journal);
1064
1065 /*
1066 * Step 3: Clear EXT4_STATE_FC_FLUSHING_DATA flag, before returning
1067 * any error from step 2. This ensures that waiters waiting on
1068 * EXT4_STATE_FC_FLUSHING_DATA can resume.
1069 */
1070 mutex_lock(&sbi->s_fc_lock);
1071 list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
1072 ext4_clear_inode_state(&iter->vfs_inode,
1073 EXT4_STATE_FC_FLUSHING_DATA);
1074 #if (BITS_PER_LONG < 64)
1075 wake_up_bit(&iter->i_state_flags, EXT4_STATE_FC_FLUSHING_DATA);
1076 #else
1077 wake_up_bit(&iter->i_flags, EXT4_STATE_FC_FLUSHING_DATA);
1078 #endif
1079 }
1080
1081 /*
1082 * Make sure clearing of EXT4_STATE_FC_FLUSHING_DATA is visible before
1083 * the waiter checks the bit. Pairs with implicit barrier in
1084 * prepare_to_wait() in ext4_fc_del().
1085 */
1086 smp_mb();
1087 mutex_unlock(&sbi->s_fc_lock);
1088
1089 /*
1090 * If we encountered error in Step 2, return it now after clearing
1091 * EXT4_STATE_FC_FLUSHING_DATA bit.
1092 */
1093 if (ret)
1094 return ret;
1095
1096
1097 /* Step 4: Mark all inodes as being committed. */
1098 jbd2_journal_lock_updates(journal);
1099 /*
1100 * The journal is now locked. No more handles can start and all the
1101 * previous handles are now drained. We now mark the inodes on the
1102 * commit queue as being committed.
1103 */
1104 mutex_lock(&sbi->s_fc_lock);
1105 list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
1106 ext4_set_inode_state(&iter->vfs_inode,
1107 EXT4_STATE_FC_COMMITTING);
1108 }
1109 mutex_unlock(&sbi->s_fc_lock);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1110 jbd2_journal_unlock_updates(journal);
1111
1112 /*
1113 * Step 5: If file system device is different from journal device,
1114 * issue a cache flush before we start writing fast commit blocks.
1115 */
1116 if (journal->j_fs_dev != journal->j_dev)
1117 blkdev_issue_flush(journal->j_fs_dev);
1118
1119 blk_start_plug(&plug);
1120 /* Step 6: Write fast commit blocks to disk. */
1121 if (sbi->s_fc_bytes == 0) {
1122 /*
1123 * Step 6.1: Add a head tag only if this is the first fast
1124 * commit in this TID.
1125 */
1126 head.fc_features = cpu_to_le32(EXT4_FC_SUPPORTED_FEATURES);
1127 head.fc_tid = cpu_to_le32(
1128 sbi->s_journal->j_running_transaction->t_tid);
1129 if (!ext4_fc_add_tlv(sb, EXT4_FC_TAG_HEAD, sizeof(head),
1130 (u8 *)&head, &crc)) {
1131 ret = -ENOSPC;
1132 goto out;
^^^^^^^^^
We dropped the lock already.
1133 }
1134 }
1135
1136 /* Step 6.2: Now write all the dentry updates. */
1137 mutex_lock(&sbi->s_fc_lock);
1138 ret = ext4_fc_commit_dentry_updates(journal, &crc);
1139 if (ret)
1140 goto out;
1141
1142 /* Step 6.3: Now write all the changed inodes to disk. */
1143 list_for_each_entry(iter, &sbi->s_fc_q[FC_Q_MAIN], i_fc_list) {
1144 inode = &iter->vfs_inode;
1145 if (!ext4_test_inode_state(inode, EXT4_STATE_FC_COMMITTING))
1146 continue;
1147
1148 ret = ext4_fc_write_inode_data(inode, &crc);
1149 if (ret)
1150 goto out;
1151 ret = ext4_fc_write_inode(inode, &crc);
1152 if (ret)
1153 goto out;
1154 }
1155 /* Step 6.4: Finally write tail tag to conclude this fast commit. */
1156 ret = ext4_fc_write_tail(sb, crc);
1157
1158 out:
--> 1159 mutex_unlock(&sbi->s_fc_lock);
Double unlock.
1160 blk_finish_plug(&plug);
1161 return ret;
1162 }
regards,
dan carpenter
Powered by blists - more mailing lists