Project

General

Profile

Bug #21025

racy is_mounted() checks in libcephfs

Added by Jeff Layton over 1 year ago. Updated 12 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Correctness/Safety
Target version:
-
Start date:
08/17/2017
Due date:
% Done:

0%

Source:
Development
Tags:
Backport:
luminous
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Component(FS):
Client, libcephfs
Labels (FS):
Pull request ID:

Description

libcephfs.cc has a bunch of is_mounted checks like this in it:

extern "C" int ceph_statfs(struct ceph_mount_info *cmount, const char *path,
                           struct statvfs *stbuf)
{
  if (!cmount->is_mounted())
    return -ENOTCONN;
  return cmount->get_client()->statfs(path, stbuf, cmount->default_perms);
}

These checks are done outside of any mutex. It's possible other calls could race with a call to ceph_unmount(). A (e.g.) ceph_statfs call could pass the is_mounted check and then end up blocked on the client_lock while ceph_umount is running. That could give very unreliable results (or possibly even crash, depending on what the call does).

Note that we will still need some sort of check in the wrappers in libcephfs.cc. We need to be able to validate that the ceph_mount_info->client pointer is valid.

All of this is should also help enable delegation support for cephfs. We'll need to be able to handle applications that don't give back their delegations in time by forcibly unmounting them. In order to do that though, we need to move the mounted checks inside the client_lock to deal with potential races conditions.


Related issues

Copied to fs - Backport #21359: luminous: racy is_mounted() checks in libcephfs Resolved

History

#1 Updated by Jeff Layton over 1 year ago

  • Subject changed from racy "mounted" checks in libcephfs to racy is_mounted() checks in libcephfs

#3 Updated by Jeff Layton over 1 year ago

  • Status changed from New to Resolved

PR is merged.

#4 Updated by Patrick Donnelly over 1 year ago

  • Category set to Correctness/Safety
  • Status changed from Resolved to Pending Backport
  • Source set to Development
  • Backport set to luminous

#5 Updated by Nathan Cutler over 1 year ago

  • Copied to Backport #21359: luminous: racy is_mounted() checks in libcephfs added

#6 Updated by Nathan Cutler 12 months ago

  • Status changed from Pending Backport to Resolved

Also available in: Atom PDF