Bug #18830: Coverity: bad iterator dereference in Locker::acquire_locks - CephFS - Ceph

Actions

Copy link

Bug #18830

closed

Coverity: bad iterator dereference in Locker::acquire_locks

Added by John Spray about 7 years ago. Updated about 7 years ago.

Status:

Resolved

Priority:

Normal

Assignee:

John Spray

Category:

Correctness/Safety

Target version:

% Done:

Source:

Tags:

Backport:

Regression:

Severity:

3 - minor

Reviewed:

Affected Versions:

ceph-qa-suite:

Component(FS):

MDS

Labels (FS):

Pull request ID:

Crash signature (v1):

Crash signature (v2):

Description

** CID 1400092:    (INVALIDATE_ITERATOR)
/home/brad/working/src/ceph/src/mds/Locker.cc: 474 in Locker::acquire_locks(boost::intrusive_ptr<MDRequestImpl> &, std::set<SimpleLock *, std::less<SimpleLock *>, std::allocator<SimpleLock *>> &, std::set<SimpleLock *, std::less<SimpleLock *>, std::allocator<SimpleLock *>> &, std::set<SimpleLock *, std::less<SimpleLock *>, std::allocator<SimpleLock *>> &, std::map<SimpleLock *, int, std::less<SimpleLock *>, std::allocator<std::pair<SimpleLock *const , int>>> *, CInode *, bool)()
/home/brad/working/src/ceph/src/mds/Locker.cc: 474 in Locker::acquire_locks(boost::intrusive_ptr<MDRequestImpl> &, std::set<SimpleLock *, std::less<SimpleLock *>, std::allocator<SimpleLock *>> &, std::set<SimpleLock *, std::less<SimpleLock *>, std::allocator<SimpleLock *>> &, std::set<SimpleLock *, std::less<SimpleLock *>, std::allocator<SimpleLock *>> &, std::map<SimpleLock *, int, std::less<SimpleLock *>, std::allocator<std::pair<SimpleLock *const , int>>> *, CInode *, bool)()

________________________________________________________________________________________________________
*** CID 1400092:    (INVALIDATE_ITERATOR)
/home/brad/working/src/ceph/src/mds/Locker.cc: 474 in Locker::acquire_locks(boost::intrusive_ptr<MDRequestImpl> &, std::set<SimpleLock *, std::less<SimpleLock *>, std::allocator<SimpleLock *>> &, std::set<SimpleLock *, std::less<SimpleLock *>, std::allocator<SimpleLock *>> &, std::set<SimpleLock *, std::less<SimpleLock *>, std::allocator<SimpleLock *>> &, std::map<SimpleLock *, int, std::less<SimpleLock *>, std::allocator<std::pair<SimpleLock *const , int>>> *, CInode *, bool)()
468             dout(10) << " already rdlocked " << *have << " " << *have->get_parent() << dendl;
469             continue;
470           }
471         }
472
473         // hose any stray locks
>>>     CID 1400092:    (INVALIDATE_ITERATOR)
>>>     Dereferencing iterator "existing" though it is already past the end of its container.
474         if (*existing == *p) {
475           assert(need_wrlock || need_remote_wrlock);
476           SimpleLock *lock = *existing;
477           if (mdr->wrlocks.count(lock)) {
478             if (!need_wrlock)
479               dout(10) << " unlocking extra " << *lock << " " << *lock->get_parent() << dendl;
/home/brad/working/src/ceph/src/mds/Locker.cc: 474 in Locker::acquire_locks(boost::intrusive_ptr<MDRequestImpl> &, std::set<SimpleLock *, std::less<SimpleLock *>, std::allocator<SimpleLock *>> &, std::set<SimpleLock *, std::less<SimpleLock *>, std::allocator<SimpleLock *>> &, std::set<SimpleLock *, std::less<SimpleLock *>, std::allocator<SimpleLock *>> &, std::map<SimpleLock *, int, std::less<SimpleLock *>, std::allocator<std::pair<SimpleLock *const , int>>> *, CInode *, bool)()
468             dout(10) << " already rdlocked " << *have << " " << *have->get_parent() << dendl;
469             continue;
470           }
471         }
472
473         // hose any stray locks
>>>     CID 1400092:    (INVALIDATE_ITERATOR)
>>>     Dereferencing iterator "existing" though it is already past the end of its container.
474         if (*existing == *p) {
475           assert(need_wrlock || need_remote_wrlock);
476           SimpleLock *lock = *existing;
477           if (mdr->wrlocks.count(lock)) {
478             if (!need_wrlock)
479               dout(10) << " unlocking extra " << *lock << " " << *lock->get_parent() << dendl;

I'm not seeing anything that prevents the ++existing in the next block taking the iterator past the end of the container, so I'm inclined to believe coverity on this one.