Hello Chuck Lever,
The patch 125b58c13f71: "NFSD: Convert the filecache to use
rhashtable" from Jun 28, 2022, leads to the following Smatch static
checker warning:
fs/nfsd/filecache.c:1117 nfsd_file_do_acquire()
warn: 'new' was already freed.
fs/nfsd/filecache.c
1066 static __be32
1067 nfsd_file_do_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
1068 unsigned int may_flags, struct nfsd_file **pnf, bool open)
1069 {
1070 struct nfsd_file_lookup_key key = {
1071 .type = NFSD_FILE_KEY_FULL,
1072 .need = may_flags & NFSD_FILE_MAY_MASK,
1073 .net = SVC_NET(rqstp),
1074 .cred = current_cred(),
1075 };
1076 struct nfsd_file *nf, *new;
1077 bool retry = true;
1078 __be32 status;
1079
1080 status = fh_verify(rqstp, fhp, S_IFREG,
1081 may_flags|NFSD_MAY_OWNER_OVERRIDE);
1082 if (status != nfs_ok)
1083 return status;
1084 key.inode = d_inode(fhp->fh_dentry);
1085
1086 retry:
1087 /* Avoid allocation if the item is already in cache */
1088 nf = rhashtable_lookup_fast(&nfsd_file_rhash_tbl, &key,
1089 nfsd_file_rhash_params);
1090 if (nf)
1091 nf = nfsd_file_get(nf);
1092 if (nf)
1093 goto wait_for_construction;
1094
1095 new = nfsd_file_alloc(&key, may_flags);
1096 if (!new) {
1097 status = nfserr_jukebox;
1098 goto out_status;
1099 }
1100
1101 nf = rhashtable_lookup_get_insert_key(&nfsd_file_rhash_tbl,
1102 &key, &new->nf_rhash,
1103 nfsd_file_rhash_params);
1104 if (!nf) {
1105 nf = new;
1106 goto open_file;
1107 }
1108 nfsd_file_slab_free(&new->nf_rcu);
^^^^^^^^^^^
This frees "new".
1109 if (IS_ERR(nf)) {
1110 trace_nfsd_file_insert_err(rqstp, key.inode, may_flags, PTR_ERR(nf));
1111 nf = NULL;
1112 status = nfserr_jukebox;
1113 goto out_status;
1114 }
1115 nf = nfsd_file_get(nf);
1116 if (nf == NULL) {
--> 1117 nf = new;
^^^
Use after free
1118 goto open_file;
1119 }
1120
1121 wait_for_construction:
1122 wait_on_bit(&nf->nf_flags, NFSD_FILE_PENDING, TASK_UNINTERRUPTIBLE);
1123
1124 /* Did construction of this file fail? */
1125 if (!test_bit(NFSD_FILE_HASHED, &nf->nf_flags)) {
1126 trace_nfsd_file_cons_err(rqstp, key.inode, may_flags, nf);
1127 if (!retry) {
1128 status = nfserr_jukebox;
1129 goto out;
1130 }
1131 retry = false;
1132 nfsd_file_put_noref(nf);
1133 goto retry;
1134 }
1135
1136 nfsd_file_lru_remove(nf);
1137 this_cpu_inc(nfsd_file_cache_hits);
1138
1139 if (!(may_flags & NFSD_MAY_NOT_BREAK_LEASE)) {
1140 bool write = (may_flags & NFSD_MAY_WRITE);
1141
1142 if (test_bit(NFSD_FILE_BREAK_READ, &nf->nf_flags) ||
1143 (test_bit(NFSD_FILE_BREAK_WRITE, &nf->nf_flags) && write)) {
1144 status = nfserrno(nfsd_open_break_lease(
1145 file_inode(nf->nf_file), may_flags));
1146 if (status == nfs_ok) {
1147 clear_bit(NFSD_FILE_BREAK_READ, &nf->nf_flags);
1148 if (write)
1149 clear_bit(NFSD_FILE_BREAK_WRITE,
1150 &nf->nf_flags);
1151 }
1152 }
1153 }
1154 out:
1155 if (status == nfs_ok) {
1156 if (open)
1157 this_cpu_inc(nfsd_file_acquisitions);
1158 *pnf = nf;
1159 } else {
1160 nfsd_file_put(nf);
1161 nf = NULL;
1162 }
1163
1164 out_status:
1165 if (open)
1166 trace_nfsd_file_acquire(rqstp, key.inode, may_flags, nf, status);
1167 return status;
1168
1169 open_file:
1170 trace_nfsd_file_alloc(nf);
1171 nf->nf_mark = nfsd_file_mark_find_or_create(nf);
1172 if (nf->nf_mark) {
1173 if (open) {
1174 status = nfsd_open_verified(rqstp, fhp, may_flags,
1175 &nf->nf_file);
1176 trace_nfsd_file_open(nf, status);
1177 } else
1178 status = nfs_ok;
1179 } else
1180 status = nfserr_jukebox;
1181 /*
1182 * If construction failed, or we raced with a call to unlink()
1183 * then unhash.
1184 */
1185 if (status != nfs_ok || key.inode->i_nlink == 0)
1186 if (nfsd_file_unhash(nf))
1187 nfsd_file_put_noref(nf);
1188 clear_bit_unlock(NFSD_FILE_PENDING, &nf->nf_flags);
1189 smp_mb__after_atomic();
1190 wake_up_bit(&nf->nf_flags, NFSD_FILE_PENDING);
1191 goto out;
1192 }
regards,
dan carpenter
