Skip to content

Commit 9e0488d

Browse files
committed
client: crash caused by invalid iterator in _readdir_cache_cb
Capacity of `readdir_cache` may change after `client_lock` is unlocked in iterations of `readdir_cache`, and it can cause the iterator to be invalid, then using the invalid iterator in the next iteration will cause crash. Crash may happen at `Dentry *dn = *pd` (pd points to invalid memory), or at `if (pd >= dir->readdir_cache.end() || *pd != dn)` (pd is smaller than begin() if idx is negative). Use index instead of iterator to solve this problem. Fixes: https://tracker.ceph.com/issues/72247 Signed-off-by: Zhansong Gao <[email protected]>
1 parent b352f56 commit 9e0488d

File tree

1 file changed

+8
-12
lines changed

1 file changed

+8
-12
lines changed

src/client/Client.cc

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9461,25 +9461,22 @@ int Client::_readdir_cache_cb(dir_result_t *dirp, add_dirent_cb_t cb, void *p,
94619461
dirp->offset, dentry_off_lt());
94629462

94639463
string dn_name;
9464-
while (true) {
9464+
for (unsigned idx = pd - dir->readdir_cache.begin();
9465+
idx < dir->readdir_cache.size();
9466+
++idx) {
94659467
int mask = caps;
94669468
if (!dirp->inode->is_complete_and_ordered())
94679469
return -EAGAIN;
9468-
if (pd == dir->readdir_cache.end())
9469-
break;
9470-
Dentry *dn = *pd;
9470+
Dentry *dn = dir->readdir_cache[idx];
94719471
if (dn->inode == NULL) {
94729472
ldout(cct, 15) << " skipping null '" << dn->name << "'" << dendl;
9473-
++pd;
94749473
continue;
94759474
}
94769475
if (dn->cap_shared_gen != dir->parent_inode->shared_gen) {
94779476
ldout(cct, 15) << " skipping mismatch shared gen '" << dn->name << "'" << dendl;
9478-
++pd;
94799477
continue;
94809478
}
94819479

9482-
int idx = pd - dir->readdir_cache.begin();
94839480
if (dn->inode->is_dir() && cct->_conf->client_dirsize_rbytes) {
94849481
mask |= CEPH_STAT_RSTAT;
94859482
}
@@ -9493,9 +9490,8 @@ int Client::_readdir_cache_cb(dir_result_t *dirp, add_dirent_cb_t cb, void *p,
94939490
return -EAGAIN;
94949491
}
94959492

9496-
// the content of readdir_cache may change after _getattr(), so pd may be invalid iterator
9497-
pd = dir->readdir_cache.begin() + idx;
9498-
if (pd >= dir->readdir_cache.end() || *pd != dn)
9493+
// the content of readdir_cache may change after _getattr()
9494+
if (idx >= dir->readdir_cache.size() || dir->readdir_cache[idx] != dn)
94999495
return -EAGAIN;
95009496

95019497
struct ceph_statx stx;
@@ -9505,8 +9501,7 @@ int Client::_readdir_cache_cb(dir_result_t *dirp, add_dirent_cb_t cb, void *p,
95059501
uint64_t next_off = dn->offset + 1;
95069502
auto dname = _unwrap_name(*diri, dn->name, dn->alternate_name);
95079503
fill_dirent(&de, dname.c_str(), stx.stx_mode, stx.stx_ino, next_off);
9508-
++pd;
9509-
if (pd == dir->readdir_cache.end())
9504+
if (idx + 1 == dir->readdir_cache.size())
95109505
next_off = dir_result_t::END;
95119506

95129507
Inode *in = NULL;
@@ -9517,6 +9512,7 @@ int Client::_readdir_cache_cb(dir_result_t *dirp, add_dirent_cb_t cb, void *p,
95179512

95189513
dn_name = dn->name; // fill in name while we have lock
95199514

9515+
// the content of readdir_cache may change after unlocking
95209516
client_lock.unlock();
95219517
r = cb(p, &de, &stx, next_off, in); // _next_ offset
95229518
client_lock.lock();

0 commit comments

Comments
 (0)