Coverity picked up some issues with the filestore code. These are mostly
old issues that appear new becuase code moved around, but this is probably
a good opportunity to fix them... :)
sage
--- Begin Message ---
Hi,
Please find the latest report on new defect(s) introduced to ceph found with Coverity Scan
Defect(s) Reported-by: Coverity Scan
Showing 7 of 9 defects
** CID 1063704: Uninitialized scalar field (UNINIT_CTOR)
/os/BtrfsFileStoreBackend.cc: 57
** CID 1063703: Time of check time of use (TOCTOU)
/os/GenericFileStoreBackend.cc: 170
** CID 1063702: Time of check time of use (TOCTOU)
/os/BtrfsFileStoreBackend.cc: 246
** CID 1063701: Copy into fixed size buffer (STRING_OVERFLOW)
/os/BtrfsFileStoreBackend.cc: 458
** CID 1063700: Copy into fixed size buffer (STRING_OVERFLOW)
/os/BtrfsFileStoreBackend.cc: 370
** CID 1063699: Resource leak (RESOURCE_LEAK)
/os/BtrfsFileStoreBackend.cc: 345
** CID 1063698: Improper use of negative value (NEGATIVE_RETURNS)
________________________________________________________________________
CID 1063704: Uninitialized scalar field (UNINIT_CTOR)
/os/BtrfsFileStoreBackend.h: 25 ( member_decl)
22 private:
23 bool has_clone_range; ///< clone range ioctl is supported
24 bool has_snap_create; ///< snap create ioctl is supported
>>> Class member declaration for "has_snap_destroy".
25 bool has_snap_destroy; ///< snap destroy ioctl is supported
26 bool has_snap_create_v2; ///< snap create v2 ioctl (async!) is supported
27 bool has_wait_sync; ///< wait sync ioctl is supported
28 bool stable_commits;
29 bool m_filestore_btrfs_clone_range;
/os/BtrfsFileStoreBackend.cc: 57 ( uninit_member)
54 GenericFileStoreBackend(fs), has_clone_range(false), has_snap_create(false),
55 has_snap_create_v2(false), has_wait_sync(false), stable_commits(false),
56 m_filestore_btrfs_clone_range(g_conf->filestore_btrfs_clone_range),
>>> CID 1063704: Uninitialized scalar field (UNINIT_CTOR)
>>> Non-static class member "has_snap_destroy" is not initialized in this constructor nor in any functions that it calls.
57 m_filestore_btrfs_snap (g_conf->filestore_btrfs_snap) { }
58
59 int BtrfsFileStoreBackend::detect_features()
60 {
61 int r;
________________________________________________________________________
CID 1063703: Time of check time of use (TOCTOU)
/os/GenericFileStoreBackend.cc: 170 ( fs_check_call)
167 int GenericFileStoreBackend::create_current()
168 {
169 struct stat st;
>>> CID 1063703: Time of check time of use (TOCTOU)
>>> Calling function "stat(char const *, stat *)" to perform check on "this->get_current_path()->c_str()".
170 int ret = ::stat(get_current_path().c_str(), &st);
171 if (ret == 0) {
172 // current/ exists
173 if (!S_ISDIR(st.st_mode)) {
174 dout(0) << "_create_current: current/ exists but is not a directory" << dendl;
/os/GenericFileStoreBackend.cc: 178 ( toctou)
175 ret = -EINVAL;
176 }
177 } else {
>>> Calling function "mkdir(char const *, __mode_t)" that uses "this->get_current_path()->c_str()" after a check function. This can cause a time-of-check, time-of-use race condition.
178 ret = ::mkdir(get_current_path().c_str(), 0755);
179 if (ret < 0) {
180 ret = -errno;
181 dout(0) << "_create_current: mkdir " << get_current_path() << " failed: "<< cpp_strerror(ret) << dendl;
182 }
________________________________________________________________________
CID 1063702: Time of check time of use (TOCTOU)
/os/BtrfsFileStoreBackend.cc: 246 ( fs_check_call)
243 int BtrfsFileStoreBackend::create_current()
244 {
245 struct stat st;
>>> CID 1063702: Time of check time of use (TOCTOU)
>>> Calling function "stat(char const *, stat *)" to perform check on "this->get_current_path()->c_str()".
246 int ret = ::stat(get_current_path().c_str(), &st);
247 if (ret == 0) {
248 // current/ exists
249 if (!S_ISDIR(st.st_mode)) {
250 dout(0) << "create_current: current/ exists but is not a directory" << dendl;
/os/BtrfsFileStoreBackend.cc: 288 ( toctou)
285 }
286
287 dout(2) << "create_current: created btrfs subvol " << get_current_path() << dendl;
>>> Calling function "chmod(char const *, __mode_t)" that uses "this->get_current_path()->c_str()" after a check function. This can cause a time-of-check, time-of-use race condition.
288 if (::chmod(get_current_path().c_str(), 0755) < 0) {
289 ret = -errno;
290 dout(0) << "create_current: failed to chmod " << get_current_path() << " to 0755: "
291 << cpp_strerror(ret) << dendl;
292 return ret;
________________________________________________________________________
CID 1063701: Copy into fixed size buffer (STRING_OVERFLOW)
/os/BtrfsFileStoreBackend.cc: 458 ( fixed_size_dest)
455 btrfs_ioctl_vol_args vol_args;
456 memset(&vol_args, 0, sizeof(vol_args));
457 vol_args.fd = 0;
>>> CID 1063701: Copy into fixed size buffer (STRING_OVERFLOW)
>>> You might overrun the 4088 byte fixed-size string "vol_args.name" by copying the return value of "std::basic_string<char, std::char_traits<char>, std::allocator<char> >::c_str() const" without checking the length.
458 strcpy(vol_args.name, name.c_str());
459
460 int ret = ::ioctl(get_basedir_fd(), BTRFS_IOC_SNAP_DESTROY, &vol_args);
461 if (ret) {
462 ret = -errno;
________________________________________________________________________
CID 1063700: Copy into fixed size buffer (STRING_OVERFLOW)
/os/BtrfsFileStoreBackend.cc: 370 ( fixed_size_dest)
367 memset(&async_args, 0, sizeof(async_args));
368 async_args.fd = get_current_fd();
369 async_args.flags = BTRFS_SUBVOL_CREATE_ASYNC;
>>> CID 1063700: Copy into fixed size buffer (STRING_OVERFLOW)
>>> You might overrun the 4040 byte fixed-size string "async_args.name" by copying the return value of "std::basic_string<char, std::char_traits<char>, std::allocator<char> >::c_str() const" without checking the length.
370 strcpy(async_args.name, name.c_str());
371
372 int r = ::ioctl(get_basedir_fd(), BTRFS_IOC_SNAP_CREATE_V2, &async_args);
373 if (r < 0) {
374 r = -errno;
________________________________________________________________________
CID 1063699: Resource leak (RESOURCE_LEAK)
/os/BtrfsFileStoreBackend.cc: 310 ( alloc_fn)
307 }
308
309 // get snap list
>>> Storage is returned from allocation function "opendir(char const *)".
310 DIR *dir = ::opendir(get_basedir_path().c_str());
311 if (!dir) {
312 ret = -errno;
313 dout(0) << "list_checkpoints: opendir '" << get_basedir_path() << "' failed: "
314 << cpp_strerror(ret) << dendl;
/os/BtrfsFileStoreBackend.cc: 310 ( var_assign)
307 }
308
309 // get snap list
>>> Assigning: "dir" = storage returned from "opendir(this->get_basedir_path()->c_str())".
310 DIR *dir = ::opendir(get_basedir_path().c_str());
311 if (!dir) {
312 ret = -errno;
313 dout(0) << "list_checkpoints: opendir '" << get_basedir_path() << "' failed: "
314 << cpp_strerror(ret) << dendl;
/os/BtrfsFileStoreBackend.cc: 321 ( noescape)
318 list<string> snaps;
319 char path[PATH_MAX];
320 struct dirent buf, *de;
>>> Resource "dir" is not freed or pointed-to in function "readdir_r(DIR *, dirent *, dirent **)".
321 while (::readdir_r(dir, &buf, &de) == 0) {
322 if (!de)
323 break;
324
325 snprintf(path, sizeof(path), "%s/%s", get_basedir_path().c_str(), de->d_name);
/os/BtrfsFileStoreBackend.cc: 321 ( noescape)
318 list<string> snaps;
319 char path[PATH_MAX];
320 struct dirent buf, *de;
>>> Resource "dir" is not freed or pointed-to in function "readdir_r(DIR *, dirent *, dirent **)".
321 while (::readdir_r(dir, &buf, &de) == 0) {
322 if (!de)
323 break;
324
325 snprintf(path, sizeof(path), "%s/%s", get_basedir_path().c_str(), de->d_name);
/os/BtrfsFileStoreBackend.cc: 345 ( leaked_storage)
342 ret = -errno;
343 dout(0) << "list_checkpoints: statfs '" << path << "' failed: "
344 << cpp_strerror(ret) << dendl;
>>> CID 1063699: Resource leak (RESOURCE_LEAK)
>>> Variable "dir" going out of scope leaks the storage it points to.
345 return ret;
346 }
347
348 if (fs.f_type == BTRFS_SUPER_MAGIC && basest.st_dev != st.st_dev)
349 snaps.push_back(string(de->d_name));
________________________________________________________________________
CID 1063698: Improper use of negative value (NEGATIVE_RETURNS)
/tools/ceph-monstore-tool.cc: 182 ( var_tested_neg)
179
180 int fd;
181 if (vm.count("out")) {
>>> Variable "fd" tests negative.
182 if ((fd = open(out_path.c_str(), O_WRONLY|O_CREAT|O_TRUNC, 0666)) == -1) {
183 int _err = errno;
184 if (_err != EISDIR) {
185 std::cerr << "Couldn't open " << out_path << ": " << cpp_strerror(_err) << std::endl;
186 return 1;
/tools/ceph-monstore-tool.cc: 236 ( negative_returns)
233 std::cerr << "Error getting map: " << cpp_strerror(r) << std::endl;
234 goto done;
235 }
>>> CID 1063698: Improper use of negative value (NEGATIVE_RETURNS)
>>> "fd" is passed to a parameter that cannot be negative.
236 bl.write_fd(fd);
237 } else if (cmd == "getosdmap") {
238 if (!store_path.size()) {
239 std::cerr << "need mon store path" << std::endl;
240 std::cerr << desc << std::endl;
________________________________________________________________________
To view the defects in Coverity Scan visit, http://scan.coverity.com
To unsubscribe from the email notification for new defects, http://scan5.coverity.com/cgi-bin/unsubscribe.py
--- End Message ---
