Hi,
static analysis with Coverity has detected an issue with the following
commit:
commit 03535b72873ba74e80e6938b5f772cf5b07ca76b
Author: Paulo Alcantara (SUSE) <pc@xxxxxx>
Date: Wed Dec 4 17:38:03 2019 -0300
cifs: Avoid doing network I/O while holding cache lock
This commit removed a memset on the vol object, causing the issue as
reported below by Coverity:
1342static struct cifs_ses *find_root_ses(struct vol_info *vi,
1343 struct cifs_tcon *tcon,
1344 const char *path)
1345{
1346 char *rpath;
1347 int rc;
1348 struct cache_entry *ce;
1349 struct dfs_info3_param ref = {0};
1350 char *mdata = NULL, *devname = NULL;
1351 struct TCP_Server_Info *server;
1352 struct cifs_ses *ses;
1. var_decl: Declaring variable vol without initializer.
1353 struct smb_vol vol;
1354
1355 rpath = get_dfs_root(path);
2. Condition IS_ERR(rpath), taking false branch.
1356 if (IS_ERR(rpath))
1357 return ERR_CAST(rpath);
1358
1359 down_read(&htable_rw_lock);
1360
1361 ce = lookup_cache_entry(rpath, NULL);
3. Condition IS_ERR(ce), taking true branch.
1362 if (IS_ERR(ce)) {
1363 up_read(&htable_rw_lock);
1364 ses = ERR_CAST(ce);
4. Jumping to label out.
1365 goto out;
1366 }
1367
1368 rc = setup_referral(path, ce, &ref, get_tgt_name(ce));
1369 if (rc) {
1370 up_read(&htable_rw_lock);
1371 ses = ERR_PTR(rc);
1372 goto out;
1373 }
1374
1375 up_read(&htable_rw_lock);
1376
1377 mdata = cifs_compose_mount_options(vi->mntdata, rpath, &ref,
1378 &devname);
1379 free_dfs_info_param(&ref);
1380
1381 if (IS_ERR(mdata)) {
1382 ses = ERR_CAST(mdata);
1383 mdata = NULL;
1384 goto out;
1385 }
1386
1387 rc = cifs_setup_volume_info(&vol, mdata, devname, false);
1388 kfree(devname);
1389
1390 if (rc) {
1391 ses = ERR_PTR(rc);
1392 goto out;
1393 }
1394
1395 server = get_tcp_server(&vol);
1396 if (!server) {
1397 ses = ERR_PTR(-EHOSTDOWN);
1398 goto out;
1399 }
1400
1401 ses = cifs_get_smb_ses(server, &vol);
1402
1403out:
5. uninit_use_in_call: Using uninitialized value vol.username when
calling cifs_cleanup_volume_info_contents.
6. uninit_use_in_call: Using uninitialized value vol.password when
calling cifs_cleanup_volume_info_contents.
7. uninit_use_in_call: Using uninitialized value vol.domainname when
calling cifs_cleanup_volume_info_contents.
8. uninit_use_in_call: Using uninitialized value vol.UNC when
calling cifs_cleanup_volume_info_contents.
9. uninit_use_in_call: Using uninitialized value vol.iocharset when
calling cifs_cleanup_volume_info_contents.
Uninitialized pointer read
10. uninit_use_in_call: Using uninitialized value vol.prepath when
calling cifs_cleanup_volume_info_contents.
1404 cifs_cleanup_volume_info_contents(&vol);
1405 kfree(mdata);
1406 kfree(rpath);
The vol object is memset as a result of calling cifs_setup_volume_info()
but this is too late for the earlier jumps to the error cleanup path at
label out. I did think about putting this back but it adds an
unnecessary memset, a better fix may be return immediately or to exit
via the kfree(mdata) at the end of the function.
Not sure what is best, so I'm flagging this up as an issue that needs
fixing.
Colin
