Skip to content

Commit

Permalink
many: fix several memory issues, fix issue with filesystem transactio…
Browse files Browse the repository at this point in the history
…ns, fix issues in SHM buffers when exporting buffers with weird offsets
  • Loading branch information
Meulengracht committed May 19, 2023
1 parent 0b8aa1b commit f7d88d4
Show file tree
Hide file tree
Showing 22 changed files with 411 additions and 264 deletions.
232 changes: 113 additions & 119 deletions kernel/handle.c

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions kernel/include/handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ AcquireHandleOfType(
*/
KERNELAPI void* KERNELABI
LookupHandleOfType(
_In_ uuid_t handleId,
_In_ HandleType_t handleType);
_In_ uuid_t ID,
_In_ HandleType_t type);

#endif //! __HANDLE_H__
12 changes: 4 additions & 8 deletions kernel/memory/heap.c
Original file line number Diff line number Diff line change
Expand Up @@ -720,8 +720,7 @@ MemoryCacheAllocate(
if (Slab->NumberOfFreeObjects == 1) {
list_remove(&Cache->PartialSlabs, Element);
}
}
else {
} else {
Element = list_front(&Cache->FreeSlabs);
assert(Element != NULL);

Expand All @@ -741,8 +740,7 @@ MemoryCacheAllocate(
Cache->NumberOfFreeObjects--;

Allocated = MEMORY_SLAB_ELEMENT(Cache, Slab, Index);
}
else if (!(Cache->Flags & HEAP_SINGLE_SLAB)) {
} else if (!(Cache->Flags & HEAP_SINGLE_SLAB)) {
Slab = __SlabCreate(Cache);
if (!Slab) {
MutexUnlock(&Cache->SyncObject);
Expand All @@ -755,15 +753,13 @@ MemoryCacheAllocate(

if (!Slab->NumberOfFreeObjects) {
list_append(&Cache->FullSlabs, &Slab->Header);
}
else {
} else {
list_append(&Cache->PartialSlabs, &Slab->Header);
Cache->NumberOfFreeObjects += (Cache->ObjectCount - 1);
}

Allocated = MEMORY_SLAB_ELEMENT(Cache, Slab, Index);
}
else {
} else {
ERROR("[heap] [%s] ran out of objects %i/%i", Cache->Name,
Cache->NumberOfFreeObjects, Cache->ObjectCount);
Allocated = NULL;
Expand Down
9 changes: 7 additions & 2 deletions kernel/memory/ms_shm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,7 @@ __UpdateMapping(
);
}

oserr_t
static oserr_t
__CreateMapping(
_In_ struct SHMBuffer* shmBuffer,
_In_ size_t offset,
Expand Down Expand Up @@ -1073,7 +1073,11 @@ __CreateMapping(
&(struct MemorySpaceMapOptions) {
.SHMTag = shmBuffer->ID,
.Pages = &shmBuffer->Pages[pageIndex],
.Length = length,
// Ensure that the length we are requesting pages for actually cover
// the asked mapping range. If we are requesting a mapping from 0xFFF0
// and 32 bytes, we will do a page-crossing, and actually we are requesting
// the entire first page.
.Length = (length + (actualOffset % pageSize)),
.Mask = shmBuffer->PageMask,
.Flags = __RecalculateFlags(shmBuffer->Flags, flags),
.PlacementFlags = MAPPING_PHYSICAL_FIXED | MAPPING_VIRTUAL_PROCESS
Expand Down Expand Up @@ -1137,6 +1141,7 @@ SHMMap(
if (oserr != OS_EOK) {
return oserr;
}
TRACE("SHMMap: mapping: 0x%p\n", mapping);

// Increase the mapping with the built-in offset
// if this was an exported buffer
Expand Down
111 changes: 109 additions & 2 deletions kernel/memory/ms_shm_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,7 @@ void TestSHMConform_IsConformed(void** state)
// needs to contain the expected setup for the virtual region
g_testContext.MemorySpaceMap.Calls[0].ExpectedSHMTag = 1; // expect 1
g_testContext.MemorySpaceMap.Calls[0].CheckSHMTag = true;
g_testContext.MemorySpaceMap.Calls[0].ExpectedLength = 0x8400;
g_testContext.MemorySpaceMap.Calls[0].ExpectedLength = (0x8400 + 0x856);
g_testContext.MemorySpaceMap.Calls[0].CheckLength = true;
g_testContext.MemorySpaceMap.Calls[0].ExpectedFlags = MAPPING_PERSISTENT | MAPPING_USERSPACE | MAPPING_READONLY;
g_testContext.MemorySpaceMap.Calls[0].CheckFlags = true;
Expand Down Expand Up @@ -1678,6 +1678,112 @@ void TestSHMMap_Simple(void** state)
TeardownTest(state);
}

static void __CreateExportedSHM(SHMHandle_t* shm, const void* buffer, size_t size)
{
int pageCount;
paddr_t* pages;
oserr_t oserr;
paddr_t startAddress = 0x1000000;

// Create page array
pageCount = (int)((size + (GetMemorySpacePageSize() - 1)) / GetMemorySpacePageSize());
pages = test_malloc(pageCount * sizeof(paddr_t));
assert_non_null(pages);

for (int i = 0; i < pageCount; i++) {
pages[i] = startAddress + (i * GetMemorySpacePageSize());
}

// The following function calls are expected during normal
// creation:

// 1. CreateHandle, nothing really we care enough to check here except
// that it gets invoked as expected. The default returned value is 1

// 2. GetMemorySpaceMapping
g_testContext.GetMemorySpaceMapping.ExpectedAddress = (vaddr_t)buffer;
g_testContext.GetMemorySpaceMapping.CheckAddress = true;
g_testContext.GetMemorySpaceMapping.ExpectedPageCount = pageCount;
g_testContext.GetMemorySpaceMapping.CheckPageCount = true;
g_testContext.GetMemorySpaceMapping.PageValues = pages;
g_testContext.GetMemorySpaceMapping.PageValuesProvided = true;
g_testContext.GetMemorySpaceMapping.ReturnValue = OS_EOK;

oserr = SHMExport(
(void*)buffer,
size,
0,
0,
shm
);

// free resources before asserting
test_free(pages);
assert_int_equal(oserr, OS_EOK);
}

void TestSHMMap_SimpleExported(void** state)
{
oserr_t oserr;
SHMHandle_t shm;
void* buffer;
int sgCount;
SHMSG_t* sg;

// Allocate a new buffer, with wierd values
buffer = (void*)0x109586;

// Create a normal buffer we can use. It must not be comitted. The SHM
// is already filled, which we don't want as we are trying to make a separate
// mapping that is not the original.
__CreateExportedSHM(&shm, buffer, 0xC888);

// Ensure new mapping
shm.Buffer = NULL;

// 1. LookupHandleOfType
g_testContext.LookupHandleOfType.ReturnValue = g_testContext.CreateHandle.Calls[0].CreatedResource;

// 2. MemorySpaceMap, this is the most interesting call to check, as that
// needs to contain the expected setup for the virtual region
g_testContext.MemorySpaceMap.Calls[0].ExpectedSHMTag = 1; // expect 1
g_testContext.MemorySpaceMap.Calls[0].CheckSHMTag = true;
g_testContext.MemorySpaceMap.Calls[0].ExpectedLength = (0xC888 + 0x586);
g_testContext.MemorySpaceMap.Calls[0].CheckLength = true;
g_testContext.MemorySpaceMap.Calls[0].ExpectedFlags = MAPPING_PERSISTENT | MAPPING_USERSPACE;
g_testContext.MemorySpaceMap.Calls[0].CheckFlags = true;
g_testContext.MemorySpaceMap.Calls[0].ExpectedPlacement = MAPPING_VIRTUAL_PROCESS | MAPPING_PHYSICAL_FIXED;
g_testContext.MemorySpaceMap.Calls[0].CheckPlacement = true;
g_testContext.MemorySpaceMap.Calls[0].ReturnedMapping = 0x20000;
g_testContext.MemorySpaceMap.Calls[0].ReturnedMappingProvided = true;
g_testContext.MemorySpaceMap.Calls[0].ReturnValue = OS_EOK;
oserr = SHMMap(
&shm,
0,
shm.Capacity,
SHM_ACCESS_READ | SHM_ACCESS_WRITE
);
assert_int_equal(oserr, OS_EOK);
assert_ptr_equal(shm.Buffer, (0x20000 + 0x586));
assert_int_equal(shm.Length, 0xC888);

// Ensure that the SG entry shows up correctly
oserr = SHMBuildSG(shm.ID, &sgCount, NULL);
assert_int_equal(oserr, OS_EOK);
assert_int_equal(sgCount, 1);

sg = test_malloc(sgCount * sizeof(SHMSG_t));
assert_non_null(sg);

oserr = SHMBuildSG(shm.ID, &sgCount, sg);
assert_int_equal(oserr, OS_EOK);
assert_int_equal(sg[0].Address, (0x1000000 + 0x586));
assert_int_equal(sg[0].Length, 0xCA7A);

test_free(sg);
TeardownTest(state);
}

void TestSHMMap_CanCommit(void** state)
{
oserr_t oserr;
Expand Down Expand Up @@ -1741,7 +1847,7 @@ void TestSHMMap_CanRemap(void** state)
// 2. MemorySpaceMap, expect a new mapping of 0x3000 when we map from page 2-4
g_testContext.MemorySpaceMap.Calls[1].ExpectedSHMTag = 1; // expect 1
g_testContext.MemorySpaceMap.Calls[1].CheckSHMTag = true;
g_testContext.MemorySpaceMap.Calls[1].ExpectedLength = 0x2890;
g_testContext.MemorySpaceMap.Calls[1].ExpectedLength = 0x3000;
g_testContext.MemorySpaceMap.Calls[1].CheckLength = true;
g_testContext.MemorySpaceMap.Calls[1].ExpectedFlags = MAPPING_PERSISTENT | MAPPING_USERSPACE;
g_testContext.MemorySpaceMap.Calls[1].CheckFlags = true;
Expand Down Expand Up @@ -1797,6 +1903,7 @@ int main(void)
cmocka_unit_test_setup(TestSHMAttach_PrivateFailed, SetupTest),
cmocka_unit_test_setup(TestSHMAttach_InvalidID, SetupTest),
cmocka_unit_test_setup(TestSHMMap_Simple, SetupTest),
cmocka_unit_test_setup(TestSHMMap_SimpleExported, SetupTest),
cmocka_unit_test_setup(TestSHMMap_CanCommit, SetupTest),
cmocka_unit_test_setup(TestSHMMap_CanRemap, SetupTest),
};
Expand Down
2 changes: 2 additions & 0 deletions kernel/sync/mutex.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ __SlowLock(
irqstate_t intStatus;
uuid_t owner;

// TODO: Detect mutex locking during IRQs

// Disable interrupts and try to acquire the lock or wait for the lock
// to unlock if it's held on another CPU - however we only wait for a brief period
intStatus = InterruptDisable();
Expand Down
66 changes: 11 additions & 55 deletions librt/libc/io/open.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#define __TRACE
//#define __TRACE
#define __need_minmax
#include <ddk/utils.h>
#include <errno.h>
Expand Down Expand Up @@ -152,7 +152,7 @@ __transfer(
_In_ size_t chunkSize,
_In_ size_t offset,
_In_ size_t length,
_Out_ size_t* bytesTransferreOut)
_Out_ size_t* bytesTransferedOut)
{
size_t bytesLeft = length;
oserr_t oserr;
Expand Down Expand Up @@ -182,7 +182,7 @@ __transfer(
offset += bytesTransferred;
}

*bytesTransferreOut = length - bytesLeft;
*bytesTransferedOut = length - bytesLeft;
return oserr;
}

Expand All @@ -194,32 +194,14 @@ __read_large(
_Out_ size_t* bytesReadOut)
{
OSHandle_t shm;
void* adjustedPointer = (void*)buffer;
size_t adjustedLength = length;
size_t pageSize = MemoryPageSize();
oserr_t oserr;
TRACE("__read_large(buffer=0x%" PRIxIN ", length=%" PRIuIN ")", buffer, length);

// enforce page alignment on the buffer
if ((uintptr_t)buffer & (pageSize - 1)) {
size_t bytesToAlign = pageSize - ((uintptr_t)buffer & (pageSize - 1));
TRACE("__read_large: aligning buffer=0x%" PRIxIN ", align=0x%" PRIxIN,
buffer, bytesToAlign);
oserr = __file_read(handle, buffer, bytesToAlign, bytesReadOut);
if (oserr != OS_EOK || *bytesReadOut == 0) {
return oserr;
}
adjustedPointer = (void*)((uintptr_t)buffer + bytesToAlign);
adjustedLength -= bytesToAlign;
}

TRACE("__read_large: exporting buffer=0x%" PRIxIN ", length=0x%" PRIxIN,
adjustedPointer, adjustedLength);
oserr = SHMExport(
adjustedPointer,
buffer,
&(SHM_t) {
.Access = SHM_ACCESS_READ | SHM_ACCESS_WRITE,
.Size = adjustedLength
.Size = length
},
&shm
);
Expand All @@ -232,15 +214,11 @@ __read_large(
handle->OSHandle.ID,
shm.ID,
false,
adjustedLength,
length,
0,
adjustedLength,
length,
bytesReadOut
);
if (*bytesReadOut == adjustedLength) {
*bytesReadOut = length;
}

OSHandleDestroy(&shm);
return oserr;
}
Expand Down Expand Up @@ -290,32 +268,13 @@ __write_large(
_Out_ size_t* bytesWrittenOut)
{
OSHandle_t shm;
void* adjustedPointer = (void*)buffer;
size_t adjustedLength = length;
size_t pageSize = MemoryPageSize();
oserr_t oserr;
TRACE("__write_large(buffer=0x%" PRIxIN ", length=%" PRIuIN ")", buffer, length);

// enforce page alignment on the buffer
if ((uintptr_t)buffer & (pageSize - 1)) {
size_t bytesToAlign = pageSize - ((uintptr_t)buffer & (pageSize - 1));
TRACE("__write_large: aligning buffer=0x%" PRIxIN ", align=0x%" PRIxIN,
buffer, bytesToAlign);
oserr = __file_write(handle, buffer, bytesToAlign, bytesWrittenOut);
if (oserr != OS_EOK|| *bytesWrittenOut == 0) {
return oserr;
}
adjustedPointer = (void*)((uintptr_t)buffer + bytesToAlign);
adjustedLength -= bytesToAlign;
}

TRACE("__write_large: exporting buffer=0x%" PRIxIN ", length=0x%" PRIxIN,
adjustedPointer, adjustedLength);
oserr = SHMExport(
adjustedPointer,
(void*)buffer,
&(SHM_t) {
.Access = SHM_ACCESS_READ | SHM_ACCESS_WRITE,
.Size = adjustedLength
.Size = length
},
&shm
);
Expand All @@ -326,15 +285,12 @@ __write_large(
oserr = __transfer(
handle->OSHandle.ID, shm.ID,
true,
adjustedLength,
length,
0,
adjustedLength,
length,
bytesWrittenOut
);
OSHandleDestroy(&shm);
if (*bytesWrittenOut == adjustedLength) {
*bytesWrittenOut = length;
}
return oserr;
}

Expand Down
2 changes: 1 addition & 1 deletion librt/libc/stdio/fread.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

//#define __TRACE
#define __need_minmax
#define __TRACE
#include <ddk/utils.h>
#include <errno.h>
#include <internal/_file.h>
Expand Down
3 changes: 1 addition & 2 deletions librt/libc/stdio/fwrite.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

//#define __TRACE
#define __need_minmax
#define __TRACE

#include <ddk/utils.h>
#include <errno.h>
#include <io.h>
Expand Down
2 changes: 1 addition & 1 deletion librt/libddk/include/ddk/barrier.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
///////////////////////////////////////////////////////////////////////////////
//// Clang
///////////////////////////////////////////////////////////////////////////////
#elif defined(__clang__)
#elif defined(__clang__) || defined(__GNUC__)
#define sw_mb() __asm__ __volatile__ ( "" ::: "memory" )
#define sw_rmb() __asm__ __volatile__ ( "" ::: "memory" )
#define sw_wmb() __asm__ __volatile__ ( "" ::: "memory" )
Expand Down
Loading

0 comments on commit f7d88d4

Please sign in to comment.