Storage: Rework LongDirEntry to be a union.

Our dirent structure is "slim": it has a flexible-length array at the
end which must be allocated to whatever size the consumer wants. However,
we use [1] there and not [0] or [], which meant GCC thought it was not
a flexible-length array, and so it optimized various string accesses
that it assumed must be always false. Among these was BDirectory's
check for "." and "..", and so that resulted in infinite loops.

When changing our dirent structure to a proper FLA instead of [1],
GCC then throws errors on LongDirEntry as it has data "after" the
FLA; which is what we want, but there is no way to tell GCC that.
So now we use a union instead, which is the proper way to statically
allocate a FLA.

This is part of #17389, but the real fix requires changing our dirent
structure, which is coming in a separate commit.
This commit is contained in:
Augustin Cavalier 2021-11-18 16:00:23 -05:00
parent 9cc1718212
commit 8f03af00f8
11 changed files with 70 additions and 54 deletions

View File

@ -25,7 +25,10 @@ namespace BPrivate {
namespace Storage {
// For convenience:
struct LongDirEntry : dirent { char _buffer[B_FILE_NAME_LENGTH]; };
union LongDirEntry {
struct dirent dirent;
char _[sizeof(struct dirent) + B_PATH_NAME_LENGTH];
};
//! Returns whether the supplied path is absolute.
bool is_absolute_path(const char *path);

View File

@ -332,19 +332,20 @@ BDirectory::GetNextRef(entry_ref* ref)
if (InitCheck() != B_OK)
return B_FILE_ERROR;
BPrivate::Storage::LongDirEntry entry;
BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent;
bool next = true;
while (next) {
if (GetNextDirents(&entry, sizeof(entry), 1) != 1)
if (GetNextDirents(entry, sizeof(longEntry), 1) != 1)
return B_ENTRY_NOT_FOUND;
next = (!strcmp(entry.d_name, ".")
|| !strcmp(entry.d_name, ".."));
next = (!strcmp(entry->d_name, ".")
|| !strcmp(entry->d_name, ".."));
}
ref->device = fDirNodeRef.device;
ref->directory = fDirNodeRef.node;
return ref->set_name(entry.d_name);
return ref->set_name(entry->d_name);
}
@ -375,11 +376,12 @@ BDirectory::CountEntries()
if (error != B_OK)
return error;
int32 count = 0;
BPrivate::Storage::LongDirEntry entry;
BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent;
while (error == B_OK) {
if (GetNextDirents(&entry, sizeof(entry), 1) != 1)
if (GetNextDirents(entry, sizeof(longEntry), 1) != 1)
break;
if (strcmp(entry.d_name, ".") != 0 && strcmp(entry.d_name, "..") != 0)
if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0)
count++;
}
Rewind();

View File

@ -109,8 +109,9 @@ BMergedDirectory::GetNextEntry(BEntry* entry, bool traverse)
status_t
BMergedDirectory::GetNextRef(entry_ref* ref)
{
BPrivate::Storage::LongDirEntry dirEntry;
int32 result = GetNextDirents(&dirEntry, sizeof(dirEntry), 1);
BPrivate::Storage::LongDirEntry longEntry;
struct dirent* dirEntry = &longEntry.dirent;
int32 result = GetNextDirents(dirEntry, sizeof(longEntry), 1);
if (result < 0)
return result;
if (result == 0)
@ -118,7 +119,7 @@ BMergedDirectory::GetNextRef(entry_ref* ref)
BEntry entry;
status_t error
= entry.SetTo(fDirectories.ItemAt(fDirectoryIndex), dirEntry.d_name);
= entry.SetTo(fDirectories.ItemAt(fDirectoryIndex), dirEntry->d_name);
if (error != B_OK)
return error;

View File

@ -309,15 +309,16 @@ BNode::GetNextAttrName(char* buffer)
if (InitAttrDir() != B_OK)
return B_FILE_ERROR;
BPrivate::Storage::LongDirEntry entry;
ssize_t result = _kern_read_dir(fAttrFd, &entry, sizeof(entry), 1);
BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent;
ssize_t result = _kern_read_dir(fAttrFd, entry, sizeof(longEntry), 1);
if (result < 0)
return result;
if (result == 0)
return B_ENTRY_NOT_FOUND;
strlcpy(buffer, entry.d_name, B_ATTR_NAME_LENGTH);
strlcpy(buffer, entry->d_name, B_ATTR_NAME_LENGTH);
return B_OK;
}

View File

@ -352,19 +352,20 @@ BDirectory::GetNextRef(entry_ref* ref)
if (InitCheck() != B_OK)
return B_FILE_ERROR;
BPrivate::Storage::LongDirEntry entry;
BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent;
bool next = true;
while (next) {
if (GetNextDirents(&entry, sizeof(entry), 1) != 1)
if (GetNextDirents(entry, sizeof(longEntry), 1) != 1)
return B_ENTRY_NOT_FOUND;
next = (!strcmp(entry.d_name, ".")
|| !strcmp(entry.d_name, ".."));
next = (!strcmp(entry->d_name, ".")
|| !strcmp(entry->d_name, ".."));
}
ref->device = entry.d_pdev;
ref->directory = entry.d_pino;
return ref->set_name(entry.d_name);
ref->device = entry->d_pdev;
ref->directory = entry->d_pino;
return ref->set_name(entry->d_name);
}
@ -395,11 +396,12 @@ BDirectory::CountEntries()
if (error != B_OK)
return error;
int32 count = 0;
BPrivate::Storage::LongDirEntry entry;
BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent;
while (error == B_OK) {
if (GetNextDirents(&entry, sizeof(entry), 1) != 1)
if (GetNextDirents(entry, sizeof(longEntry), 1) != 1)
break;
if (strcmp(entry.d_name, ".") != 0 && strcmp(entry.d_name, "..") != 0)
if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0)
count++;
}
Rewind();

View File

@ -111,16 +111,17 @@ BMergedDirectory::GetNextEntry(BEntry* entry, bool traverse)
status_t
BMergedDirectory::GetNextRef(entry_ref* ref)
{
BPrivate::Storage::LongDirEntry entry;
int32 result = GetNextDirents(&entry, sizeof(entry), 1);
BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent;
int32 result = GetNextDirents(entry, sizeof(longEntry), 1);
if (result < 0)
return result;
if (result == 0)
return B_ENTRY_NOT_FOUND;
ref->device = entry.d_pdev;
ref->directory = entry.d_pino;
return ref->set_name(entry.d_name);
ref->device = entry->d_pdev;
ref->directory = entry->d_pino;
return ref->set_name(entry->d_name);
}

View File

@ -319,15 +319,16 @@ BNode::GetNextAttrName(char* buffer)
if (InitAttrDir() != B_OK)
return B_FILE_ERROR;
BPrivate::Storage::LongDirEntry entry;
ssize_t result = _kern_read_dir(fAttrFd, &entry, sizeof(entry), 1);
BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent;
ssize_t result = _kern_read_dir(fAttrFd, entry, sizeof(longEntry), 1);
if (result < 0)
return result;
if (result == 0)
return B_ENTRY_NOT_FOUND;
strlcpy(buffer, entry.d_name, B_ATTR_NAME_LENGTH);
strlcpy(buffer, entry->d_name, B_ATTR_NAME_LENGTH);
return B_OK;
}

View File

@ -348,20 +348,21 @@ BQuery::GetNextRef(entry_ref* ref)
if (error == B_OK && !_HasFetched())
error = B_FILE_ERROR;
if (error == B_OK) {
BPrivate::Storage::LongDirEntry entry;
BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent;
bool next = true;
while (error == B_OK && next) {
if (GetNextDirents(&entry, sizeof(entry), 1) != 1) {
if (GetNextDirents(entry, sizeof(longEntry), 1) != 1) {
error = B_ENTRY_NOT_FOUND;
} else {
next = (!strcmp(entry.d_name, ".")
|| !strcmp(entry.d_name, ".."));
next = (!strcmp(entry->d_name, ".")
|| !strcmp(entry->d_name, ".."));
}
}
if (error == B_OK) {
ref->device = entry.d_pdev;
ref->directory = entry.d_pino;
error = ref->set_name(entry.d_name);
ref->device = entry->d_pdev;
ref->directory = entry->d_pino;
error = ref->set_name(entry->d_name);
}
}
return error;

View File

@ -82,16 +82,17 @@ VirtualDirectoryEntryList::GetNextEntry(BEntry* entry, bool traverse)
status_t
VirtualDirectoryEntryList::GetNextRef(entry_ref* ref)
{
BPrivate::Storage::LongDirEntry entry;
int32 result = GetNextDirents(&entry, sizeof(entry), 1);
BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent;
int32 result = GetNextDirents(entry, sizeof(longEntry), 1);
if (result < 0)
return result;
if (result == 0)
return B_ENTRY_NOT_FOUND;
ref->device = entry.d_pdev;
ref->directory = entry.d_pino;
return ref->set_name(entry.d_name);
ref->device = entry->d_pdev;
ref->directory = entry->d_pino;
return ref->set_name(entry->d_name);
}

View File

@ -208,14 +208,15 @@ VirtualDirectoryPoseView::_EntryCreated(const BMessage* message)
if (directory.SetTo(&nodeRef) != B_OK)
return true;
BPrivate::Storage::LongDirEntry entry;
while (directory.GetNextDirents(&entry, sizeof(entry), 1) == 1) {
if (strcmp(entry.d_name, ".") != 0
&& strcmp(entry.d_name, "..") != 0) {
BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent;
while (directory.GetNextDirents(entry, sizeof(longEntry), 1) == 1) {
if (strcmp(entry->d_name, ".") != 0
&& strcmp(entry->d_name, "..") != 0) {
_DispatchEntryCreatedOrRemovedMessage(B_ENTRY_CREATED,
node_ref(entry.d_dev, entry.d_ino),
NotOwningEntryRef(entry.d_pdev, entry.d_pino,
entry.d_name),
node_ref(entry->d_dev, entry->d_ino),
NotOwningEntryRef(entry->d_pdev, entry->d_pino,
entry->d_name),
NULL, false);
}
}

View File

@ -42,8 +42,10 @@ using namespace boot;
struct __DIR {
Directory* directory;
void* cookie;
dirent entry;
char nameBuffer[B_FILE_NAME_LENGTH - 1];
union {
dirent entry;
char nameBuffer[sizeof(dirent) + B_FILE_NAME_LENGTH - 1];
};
};