Hello Ronnie Sahlberg,
The patch ab2ef9a696a6: "cifs: find and use the dentry for cached
non-root directories also" from Oct 12, 2022, leads to the following
Smatch static checker warning:
fs/cifs/cached_dir.c:257 open_cached_dir()
warn: missing error code here? 'IS_ERR()' failed. 'rc' = '0'
fs/cifs/cached_dir.c
105 int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
106 const char *path,
107 struct cifs_sb_info *cifs_sb,
108 bool lookup_only, struct cached_fid **ret_cfid)
109 {
110 struct cifs_ses *ses;
111 struct TCP_Server_Info *server;
112 struct cifs_open_parms oparms;
113 struct smb2_create_rsp *o_rsp = NULL;
114 struct smb2_query_info_rsp *qi_rsp = NULL;
115 int resp_buftype[2];
116 struct smb_rqst rqst[2];
117 struct kvec rsp_iov[2];
118 struct kvec open_iov[SMB2_CREATE_IOV_SIZE];
119 struct kvec qi_iov[1];
120 int rc, flags = 0;
121 __le16 *utf16_path = NULL;
122 u8 oplock = SMB2_OPLOCK_LEVEL_II;
123 struct cifs_fid *pfid;
124 struct dentry *dentry = NULL;
125 struct cached_fid *cfid;
126 struct cached_fids *cfids;
127
128 if (tcon == NULL || tcon->cfids == NULL || tcon->nohandlecache ||
129 is_smb1_server(tcon->ses->server))
130 return -EOPNOTSUPP;
131
132 ses = tcon->ses;
133 server = ses->server;
134 cfids = tcon->cfids;
135
136 if (!server->ops->new_lease_key)
137 return -EIO;
138
139 if (cifs_sb->root == NULL)
140 return -ENOENT;
141
142 utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
143 if (!utf16_path)
144 return -ENOMEM;
145
146 cfid = find_or_create_cached_dir(cfids, path, lookup_only);
147 if (cfid == NULL) {
148 kfree(utf16_path);
149 return -ENOENT;
150 }
151 /*
152 * At this point we either have a lease already and we can just
153 * return it. If not we are guaranteed to be the only thread accessing
154 * this cfid.
155 */
156 if (cfid->has_lease) {
157 *ret_cfid = cfid;
158 kfree(utf16_path);
159 return 0;
160 }
161
162 /*
163 * We do not hold the lock for the open because in case
164 * SMB2_open needs to reconnect.
165 * This is safe because no other thread will be able to get a ref
166 * to the cfid until we have finished opening the file and (possibly)
167 * acquired a lease.
168 */
169 if (smb3_encryption_required(tcon))
170 flags |= CIFS_TRANSFORM_REQ;
171
172 pfid = &cfid->fid;
173 server->ops->new_lease_key(pfid);
174
175 memset(rqst, 0, sizeof(rqst));
176 resp_buftype[0] = resp_buftype[1] = CIFS_NO_BUFFER;
177 memset(rsp_iov, 0, sizeof(rsp_iov));
178
179 /* Open */
180 memset(&open_iov, 0, sizeof(open_iov));
181 rqst[0].rq_iov = open_iov;
182 rqst[0].rq_nvec = SMB2_CREATE_IOV_SIZE;
183
184 oparms.tcon = tcon;
185 oparms.create_options = cifs_create_options(cifs_sb, CREATE_NOT_FILE);
186 oparms.desired_access = FILE_READ_ATTRIBUTES;
187 oparms.disposition = FILE_OPEN;
188 oparms.fid = pfid;
189 oparms.reconnect = false;
190
191 rc = SMB2_open_init(tcon, server,
192 &rqst[0], &oplock, &oparms, utf16_path);
193 if (rc)
194 goto oshr_free;
195 smb2_set_next_command(tcon, &rqst[0]);
196
197 memset(&qi_iov, 0, sizeof(qi_iov));
198 rqst[1].rq_iov = qi_iov;
199 rqst[1].rq_nvec = 1;
200
201 rc = SMB2_query_info_init(tcon, server,
202 &rqst[1], COMPOUND_FID,
203 COMPOUND_FID, FILE_ALL_INFORMATION,
204 SMB2_O_INFO_FILE, 0,
205 sizeof(struct smb2_file_all_info) +
206 PATH_MAX * 2, 0, NULL);
207 if (rc)
208 goto oshr_free;
209
210 smb2_set_related(&rqst[1]);
211
212 rc = compound_send_recv(xid, ses, server,
213 flags, 2, rqst,
214 resp_buftype, rsp_iov);
215 if (rc) {
216 if (rc == -EREMCHG) {
217 tcon->need_reconnect = true;
218 pr_warn_once("server share %s deleted\n",
219 tcon->tree_name);
220 }
221 goto oshr_free;
222 }
223
224 atomic_inc(&tcon->num_remote_opens);
225
226 o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
227 oparms.fid->persistent_fid = o_rsp->PersistentFileId;
228 oparms.fid->volatile_fid = o_rsp->VolatileFileId;
229 #ifdef CONFIG_CIFS_DEBUG2
230 oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId);
231 #endif /* CIFS_DEBUG2 */
232
233 if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE)
234 goto oshr_free;
235
236
237 smb2_parse_contexts(server, o_rsp,
238 &oparms.fid->epoch,
239 oparms.fid->lease_key, &oplock,
240 NULL, NULL);
241
242 qi_rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base;
243 if (le32_to_cpu(qi_rsp->OutputBufferLength) < sizeof(struct smb2_file_all_info))
244 goto oshr_free;
245 if (!smb2_validate_and_copy_iov(
246 le16_to_cpu(qi_rsp->OutputBufferOffset),
247 sizeof(struct smb2_file_all_info),
248 &rsp_iov[1], sizeof(struct smb2_file_all_info),
249 (char *)&cfid->file_all_info))
250 cfid->file_all_info_is_valid = true;
251
252 if (!path[0])
253 dentry = dget(cifs_sb->root);
254 else {
255 dentry = path_to_dentry(cifs_sb, path);
256 if (IS_ERR(dentry))
--> 257 goto oshr_free;
Should this be an error path? There is a lot going on in the cleanup
code so maybe the error code is set later.
258 }
259 cfid->dentry = dentry;
260 cfid->tcon = tcon;
261 cfid->time = jiffies;
262 cfid->is_open = true;
263 cfid->has_lease = true;
264
265 oshr_free:
266 kfree(utf16_path);
267 SMB2_open_free(&rqst[0]);
268 SMB2_query_info_free(&rqst[1]);
269 free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
270 free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
271 spin_lock(&cfids->cfid_list_lock);
272 if (!cfid->has_lease) {
273 if (cfid->on_list) {
274 list_del(&cfid->entry);
275 cfid->on_list = false;
276 cfids->num_entries--;
277 }
278 rc = -ENOENT;
279 }
280 spin_unlock(&cfids->cfid_list_lock);
281 if (rc) {
282 free_cached_dir(cfid);
283 cfid = NULL;
284 }
285
286 if (rc == 0)
287 *ret_cfid = cfid;
288
289 return rc;
290 }
regards,
dan carpenter
