[bug report] NFSD: Convert the filecache to use rhashtable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux