nfs4: Fix problems with reading directory entries

* Inode::ReadDir missed an entry when given buffer was too small
 * Each OpenDirCookie now has its own copy of directory snapshot what
   would prevent Inode::ReadDir from accessing freed memory when removing
   files and reading directory entries simultaneously
 * Several minor issues fixed
This commit is contained in:
Pawel Dziepak
2012-12-19 08:12:39 +01:00
parent aec9dfc921
commit eed5b716d0
7 changed files with 51 additions and 18 deletions
@@ -25,6 +25,14 @@ NameCacheEntry::NameCacheEntry(const char* name, ino_t node)
} }
NameCacheEntry::NameCacheEntry(const NameCacheEntry& entry)
:
fNode(entry.fNode),
fName(strdup(entry.fName))
{
}
NameCacheEntry::~NameCacheEntry() NameCacheEntry::~NameCacheEntry()
{ {
free(const_cast<char*>(fName)); free(const_cast<char*>(fName));
@@ -37,6 +45,26 @@ DirectoryCacheSnapshot::DirectoryCacheSnapshot()
} }
DirectoryCacheSnapshot::DirectoryCacheSnapshot(
const DirectoryCacheSnapshot& snapshot)
{
mutex_init(&fLock, NULL);
MutexLocker _(snapshot.fLock);
NameCacheEntry* entry = snapshot.fEntries.Head();
NameCacheEntry* new_entry;
while (entry) {
new_entry = new NameCacheEntry(*entry);
if (new_entry == NULL)
break;
fEntries.Add(new_entry);
entry = snapshot.fEntries.GetNext(entry);
}
}
DirectoryCacheSnapshot::~DirectoryCacheSnapshot() DirectoryCacheSnapshot::~DirectoryCacheSnapshot()
{ {
while (!fEntries.IsEmpty()) { while (!fEntries.IsEmpty()) {
@@ -24,14 +24,17 @@ struct NameCacheEntry :
const char* fName; const char* fName;
NameCacheEntry(const char* name, ino_t node); NameCacheEntry(const char* name, ino_t node);
NameCacheEntry(const NameCacheEntry& entry);
~NameCacheEntry(); ~NameCacheEntry();
}; };
struct DirectoryCacheSnapshot : public KernelReferenceable { struct DirectoryCacheSnapshot : public KernelReferenceable {
SinglyLinkedList<NameCacheEntry> fEntries; SinglyLinkedList<NameCacheEntry> fEntries;
mutex fLock; mutable mutex fLock;
DirectoryCacheSnapshot(); DirectoryCacheSnapshot();
DirectoryCacheSnapshot(
const DirectoryCacheSnapshot& snapshot);
~DirectoryCacheSnapshot(); ~DirectoryCacheSnapshot();
}; };
@@ -250,7 +250,7 @@ Inode::Link(Inode* dir, const char* name)
&& dir->fCache->ChangeInfo() == changeInfo.fBefore) { && dir->fCache->ChangeInfo() == changeInfo.fBefore) {
dir->fCache->AddEntry(name, fInfo.fFileId, true); dir->fCache->AddEntry(name, fInfo.fFileId, true);
dir->fCache->SetChangeInfo(changeInfo.fAfter); dir->fCache->SetChangeInfo(changeInfo.fAfter);
} else if (dir->fCache->ChangeInfo() != changeInfo.fBefore) } else
dir->fCache->Trash(); dir->fCache->Trash();
} }
dir->fCache->Unlock(); dir->fCache->Unlock();
@@ -428,7 +428,7 @@ Inode::CreateObject(const char* name, const char* path, int mode, FileType type,
if (changeInfo.fAtomic && fCache->ChangeInfo() == changeInfo.fBefore) { if (changeInfo.fAtomic && fCache->ChangeInfo() == changeInfo.fBefore) {
fCache->AddEntry(name, fileID, true); fCache->AddEntry(name, fileID, true);
fCache->SetChangeInfo(changeInfo.fAfter); fCache->SetChangeInfo(changeInfo.fAfter);
} else if (fCache->ChangeInfo() != changeInfo.fBefore) } else
fCache->Trash(); fCache->Trash();
} }
fCache->Unlock(); fCache->Unlock();
@@ -123,7 +123,7 @@ Inode::FillDirEntry(struct dirent* de, ino_t id, const char* name, uint32 pos,
ASSERT(de != NULL); ASSERT(de != NULL);
ASSERT(name != NULL); ASSERT(name != NULL);
uint32 nameSize = strlen(name); uint32 nameSize = strlen(name) + 1;
const uint32 entSize = sizeof(struct dirent); const uint32 entSize = sizeof(struct dirent);
if (pos + entSize + nameSize > size) if (pos + entSize + nameSize > size)
@@ -282,7 +282,7 @@ Inode::GetDirSnapshot(DirectoryCacheSnapshot** _snapshot,
free(const_cast<char*>(name)); free(const_cast<char*>(name));
if (entry == NULL || entry->fName == NULL) { if (entry == NULL || entry->fName == NULL) {
if (entry->fName == NULL) if (entry != NULL)
delete entry; delete entry;
delete snapshot; delete snapshot;
delete[] dirents; delete[] dirents;
@@ -324,10 +324,10 @@ Inode::ReadDir(void* _buffer, uint32 size, uint32* _count,
else else
fFileSystem->Revalidator().RemoveDirectory(cache); fFileSystem->Revalidator().RemoveDirectory(cache);
cookie->fSnapshot = cache->GetSnapshot(); DirectoryCacheSnapshot* snapshot = cache->GetSnapshot();
if (cookie->fSnapshot == NULL) { if (snapshot == NULL) {
uint64 change; uint64 change;
result = GetDirSnapshot(&cookie->fSnapshot, cookie, &change, result = GetDirSnapshot(&snapshot, cookie, &change,
cookie->fAttrDir); cookie->fAttrDir);
if (result != B_OK) { if (result != B_OK) {
cache->Unlock(); cache->Unlock();
@@ -335,9 +335,9 @@ Inode::ReadDir(void* _buffer, uint32 size, uint32* _count,
return result; return result;
} }
cache->ValidateChangeInfo(change); cache->ValidateChangeInfo(change);
cache->SetSnapshot(cookie->fSnapshot); cache->SetSnapshot(snapshot);
} }
cookie->fSnapshot->AcquireReference(); cookie->fSnapshot = new DirectoryCacheSnapshot(*snapshot);
fFileSystem->Revalidator().AddDirectory(cache); fFileSystem->Revalidator().AddDirectory(cache);
cache->Unlock(); cache->Unlock();
fFileSystem->Revalidator().Unlock(); fFileSystem->Revalidator().Unlock();
@@ -387,6 +387,7 @@ Inode::ReadDir(void* _buffer, uint32 size, uint32* _count,
MutexLocker _(cookie->fSnapshot->fLock); MutexLocker _(cookie->fSnapshot->fLock);
for (; !overflow && i < *_count; i++) { for (; !overflow && i < *_count; i++) {
struct dirent* de = reinterpret_cast<dirent*>(buffer + pos); struct dirent* de = reinterpret_cast<dirent*>(buffer + pos);
NameCacheEntry* temp = cookie->fCurrent;
if (cookie->fCurrent == NULL) if (cookie->fCurrent == NULL)
cookie->fCurrent = cookie->fSnapshot->fEntries.Head(); cookie->fCurrent = cookie->fSnapshot->fEntries.Head();
@@ -402,6 +403,7 @@ Inode::ReadDir(void* _buffer, uint32 size, uint32* _count,
if (FillDirEntry(de, cookie->fCurrent->fNode, cookie->fCurrent->fName, if (FillDirEntry(de, cookie->fCurrent->fNode, cookie->fCurrent->fName,
pos, size) == B_BUFFER_OVERFLOW) { pos, size) == B_BUFFER_OVERFLOW) {
cookie->fCurrent = temp;
overflow = true; overflow = true;
break; break;
} }
@@ -52,7 +52,7 @@ Inode::CreateState(const char* name, int mode, int perms, OpenState* state,
&& fCache->ChangeInfo() == changeInfo.fBefore) { && fCache->ChangeInfo() == changeInfo.fBefore) {
fCache->AddEntry(name, fileID, true); fCache->AddEntry(name, fileID, true);
fCache->SetChangeInfo(changeInfo.fAfter); fCache->SetChangeInfo(changeInfo.fAfter);
} else if (fCache->ChangeInfo() != changeInfo.fBefore) } else
fCache->Trash(); fCache->Trash();
} }
fCache->Unlock(); fCache->Unlock();
@@ -421,10 +421,7 @@ ReplyInterpreter::ReadDir(uint64* cookie, uint64* cookieVerf,
isNext = fReply->Stream().GetBoolean(); isNext = fReply->Stream().GetBoolean();
} }
if (!isNext)
*eof = fReply->Stream().GetBoolean(); *eof = fReply->Stream().GetBoolean();
else
*eof = false;
*_count = count; *_count = count;
*dirents = entries; *dirents = entries;
@@ -1039,6 +1039,9 @@ nfs4_rewind_dir(fs_volume* volume, fs_vnode* vnode, void* _cookie)
OpenDirCookie* cookie = reinterpret_cast<OpenDirCookie*>(_cookie); OpenDirCookie* cookie = reinterpret_cast<OpenDirCookie*>(_cookie);
cookie->fSpecial = 0; cookie->fSpecial = 0;
if (cookie->fSnapshot != NULL)
cookie->fSnapshot->ReleaseReference();
cookie->fSnapshot = NULL;
cookie->fCurrent = NULL; cookie->fCurrent = NULL;
cookie->fEOF = false; cookie->fEOF = false;