From a31dd7b4547547c30f2cc513babe216bc3e8c5b7 Mon Sep 17 00:00:00 2001 From: Mateusz Jablonski Date: Mon, 9 Oct 2023 09:30:26 +0000 Subject: [PATCH] fix: add unrecoverable to avoid out of bound access Related-To: NEO-9038 Signed-off-by: Mateusz Jablonski --- .../os_interface/linux/drm_memory_manager.cpp | 9 ++++++--- .../os_interface/linux/drm_memory_manager.h | 1 - shared/source/os_interface/linux/sys_calls.h | 1 + .../os_interface/linux/sys_calls_linux.cpp | 3 +++ .../mocks/linux/mock_drm_memory_manager.cpp | 8 -------- .../mocks/linux/mock_drm_memory_manager.h | 9 --------- .../os_interface/linux/sys_calls_linux_ult.cpp | 7 +++++++ .../os_interface/linux/sys_calls_linux_ult.h | 4 ++++ .../linux/drm_memory_manager_tests.cpp | 17 +++++++++++------ 9 files changed, 32 insertions(+), 27 deletions(-) diff --git a/shared/source/os_interface/linux/drm_memory_manager.cpp b/shared/source/os_interface/linux/drm_memory_manager.cpp index 7ae60bfb02..822bf4b3c6 100644 --- a/shared/source/os_interface/linux/drm_memory_manager.cpp +++ b/shared/source/os_interface/linux/drm_memory_manager.cpp @@ -41,6 +41,7 @@ #include "shared/source/os_interface/linux/i915_prelim.h" #include "shared/source/os_interface/linux/memory_info.h" #include "shared/source/os_interface/linux/os_context_linux.h" +#include "shared/source/os_interface/linux/sys_calls.h" #include "shared/source/os_interface/os_interface.h" #include @@ -815,7 +816,7 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromMultipleShared if (bo == nullptr) { areBosSharedObjects = false; - size_t size = lseekFunction(handle, 0, SEEK_END); + size_t size = SysCalls::lseek(handle, 0, SEEK_END); UNRECOVERABLE_IF(size == std::numeric_limits::max()); totalSize += size; @@ -964,7 +965,8 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromSharedHandle(o } if (bo == nullptr) { - size_t size = lseekFunction(handle, 0, SEEK_END); + size_t size = SysCalls::lseek(handle, 0, SEEK_END); + UNRECOVERABLE_IF(size == std::numeric_limits::max()); auto patIndex = drm.getPatIndex(nullptr, properties.allocationType, CacheRegion::Default, CachePolicy::WriteBack, false); auto boHandleWrapper = reuseSharedAllocation ? BufferObjectHandleWrapper{boHandle} : tryToGetBoHandleWrapperWithSharedOwnership(boHandle); @@ -2388,7 +2390,8 @@ DrmAllocation *DrmMemoryManager::createUSMHostAllocationFromSharedHandle(osHandl if (bo == nullptr) { void *cpuPointer = nullptr; - size_t size = lseekFunction(handle, 0, SEEK_END); + size_t size = SysCalls::lseek(handle, 0, SEEK_END); + UNRECOVERABLE_IF(size == std::numeric_limits::max()); bo = new BufferObject(properties.rootDeviceIndex, &drm, patIndex, boHandle, size, maxOsContextCount); diff --git a/shared/source/os_interface/linux/drm_memory_manager.h b/shared/source/os_interface/linux/drm_memory_manager.h index e86b0f6bb3..f49b322f8e 100644 --- a/shared/source/os_interface/linux/drm_memory_manager.h +++ b/shared/source/os_interface/linux/drm_memory_manager.h @@ -161,7 +161,6 @@ class DrmMemoryManager : public MemoryManager { std::unique_ptr gemCloseWorker; decltype(&mmap) mmapFunction = mmap; decltype(&munmap) munmapFunction = munmap; - decltype(&lseek) lseekFunction = lseek; decltype(&close) closeFunction = close; std::vector sharingBufferObjects; std::mutex mtx; diff --git a/shared/source/os_interface/linux/sys_calls.h b/shared/source/os_interface/linux/sys_calls.h index 53985cb3eb..43b7a9d8da 100644 --- a/shared/source/os_interface/linux/sys_calls.h +++ b/shared/source/os_interface/linux/sys_calls.h @@ -51,5 +51,6 @@ int unlink(const std::string &pathname); DIR *opendir(const char *name); struct dirent *readdir(DIR *dir); int closedir(DIR *dir); +off_t lseek(int fd, off_t offset, int whence) noexcept; } // namespace SysCalls } // namespace NEO diff --git a/shared/source/os_interface/linux/sys_calls_linux.cpp b/shared/source/os_interface/linux/sys_calls_linux.cpp index 905786e751..c0e74a5e10 100644 --- a/shared/source/os_interface/linux/sys_calls_linux.cpp +++ b/shared/source/os_interface/linux/sys_calls_linux.cpp @@ -183,5 +183,8 @@ int closedir(DIR *dir) { return ::closedir(dir); } +off_t lseek(int fd, off_t offset, int whence) noexcept { + return ::lseek(fd, offset, whence); +} } // namespace SysCalls } // namespace NEO diff --git a/shared/test/common/mocks/linux/mock_drm_memory_manager.cpp b/shared/test/common/mocks/linux/mock_drm_memory_manager.cpp index 5360c34bd4..2e79ddfdd7 100644 --- a/shared/test/common/mocks/linux/mock_drm_memory_manager.cpp +++ b/shared/test/common/mocks/linux/mock_drm_memory_manager.cpp @@ -19,8 +19,6 @@ #include namespace NEO { -off_t lseekReturn = 4096u; -std::atomic lseekCalledCount(0); std::atomic closeInputFd(0); std::atomic closeCalledCount(0); @@ -30,10 +28,7 @@ TestedDrmMemoryManager::TestedDrmMemoryManager(ExecutionEnvironment &executionEn executionEnvironment) { this->mmapFunction = SysCalls::mmap; this->munmapFunction = SysCalls::munmap; - this->lseekFunction = &lseekMock; this->closeFunction = &closeMock; - lseekReturn = 4096; - lseekCalledCount = 0; closeInputFd = 0; closeCalledCount = 0; hostPtrManager.reset(new MockHostPtrManager); @@ -49,10 +44,7 @@ TestedDrmMemoryManager::TestedDrmMemoryManager(bool enableLocalMemory, executionEnvironment) { this->mmapFunction = SysCalls::mmap; this->munmapFunction = SysCalls::munmap; - this->lseekFunction = &lseekMock; this->closeFunction = &closeMock; - lseekReturn = 4096; - lseekCalledCount = 0; closeInputFd = 0; closeCalledCount = 0; this->executionEnvironment = &executionEnvironment; diff --git a/shared/test/common/mocks/linux/mock_drm_memory_manager.h b/shared/test/common/mocks/linux/mock_drm_memory_manager.h index 54ecef9738..73c6dafa1d 100644 --- a/shared/test/common/mocks/linux/mock_drm_memory_manager.h +++ b/shared/test/common/mocks/linux/mock_drm_memory_manager.h @@ -15,18 +15,9 @@ #include namespace NEO { -extern off_t lseekReturn; -extern std::atomic lseekCalledCount; extern std::atomic closeInputFd; extern std::atomic closeCalledCount; -inline off_t lseekMock(int fd, off_t offset, int whence) noexcept { - lseekCalledCount++; - if ((fd == closeInputFd) && (closeCalledCount > 0)) { - return 0; - } - return lseekReturn; -} inline int closeMock(int fd) { closeInputFd = fd; closeCalledCount++; diff --git a/shared/test/common/os_interface/linux/sys_calls_linux_ult.cpp b/shared/test/common/os_interface/linux/sys_calls_linux_ult.cpp index 445d91983e..6f63d9dd9b 100644 --- a/shared/test/common/os_interface/linux/sys_calls_linux_ult.cpp +++ b/shared/test/common/os_interface/linux/sys_calls_linux_ult.cpp @@ -101,6 +101,8 @@ DIR *(*sysCallsOpendir)(const char *name) = nullptr; struct dirent *(*sysCallsReaddir)(DIR *dir) = nullptr; int (*sysCallsClosedir)(DIR *dir) = nullptr; int (*sysCallsGetDevicePath)(int deviceFd, char *buf, size_t &bufSize) = nullptr; +off_t lseekReturn = 4096u; +std::atomic lseekCalledCount(0); int mkdir(const std::string &path) { if (sysCallsMkdir != nullptr) { @@ -457,5 +459,10 @@ int closedir(DIR *dir) { return 0; } +off_t lseek(int fd, off_t offset, int whence) noexcept { + lseekCalledCount++; + return lseekReturn; +} + } // namespace SysCalls } // namespace NEO diff --git a/shared/test/common/os_interface/linux/sys_calls_linux_ult.h b/shared/test/common/os_interface/linux/sys_calls_linux_ult.h index 8a379abe19..3d441f41e1 100644 --- a/shared/test/common/os_interface/linux/sys_calls_linux_ult.h +++ b/shared/test/common/os_interface/linux/sys_calls_linux_ult.h @@ -7,6 +7,7 @@ #pragma once +#include #include #include #include @@ -73,5 +74,8 @@ extern bool mmapCaptureExtendedPointers; extern bool mmapAllowExtendedPointers; extern uint32_t mmapFuncCalled; extern uint32_t munmapFuncCalled; + +extern off_t lseekReturn; +extern std::atomic lseekCalledCount; } // namespace SysCalls } // namespace NEO diff --git a/shared/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp b/shared/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp index 991f1525f7..b887dad627 100644 --- a/shared/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp +++ b/shared/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp @@ -2029,6 +2029,7 @@ TEST_F(DrmMemoryManagerTest, given32BitAddressingWhenBufferFromSharedHandleAndBi mock->ioctlExpected.gemWait = 1; mock->ioctlExpected.gemClose = 1; + SysCalls::lseekCalledCount = 0; memoryManager->setForce32BitAllocations(true); osHandle handle = 1u; this->mock->outputHandle = 2u; @@ -2038,7 +2039,7 @@ TEST_F(DrmMemoryManagerTest, given32BitAddressingWhenBufferFromSharedHandleAndBi auto graphicsAllocation = memoryManager->createGraphicsAllocationFromSharedHandle(handle, properties, true, false, true, nullptr); auto drmAllocation = static_cast(graphicsAllocation); EXPECT_TRUE(graphicsAllocation->is32BitAllocation()); - EXPECT_EQ(1, lseekCalledCount); + EXPECT_EQ(1, SysCalls::lseekCalledCount); auto gmmHelper = device->getGmmHelper(); EXPECT_EQ(gmmHelper->canonize(memoryManager->getExternalHeapBaseAddress(graphicsAllocation->getRootDeviceIndex(), drmAllocation->isAllocatedInLocalMemoryPool())), drmAllocation->getGpuBaseAddress()); memoryManager->freeGraphicsMemory(graphicsAllocation); @@ -2048,6 +2049,7 @@ TEST_F(DrmMemoryManagerTest, given32BitAddressingWhenBufferFromSharedHandleIsCre mock->ioctlExpected.primeFdToHandle = 1; mock->ioctlExpected.gemWait = 1; mock->ioctlExpected.gemClose = 1; + SysCalls::lseekCalledCount = 0; memoryManager->setForce32BitAllocations(true); osHandle handle = 1u; @@ -2057,7 +2059,7 @@ TEST_F(DrmMemoryManagerTest, given32BitAddressingWhenBufferFromSharedHandleIsCre auto drmAllocation = static_cast(graphicsAllocation); EXPECT_FALSE(graphicsAllocation->is32BitAllocation()); - EXPECT_EQ(1, lseekCalledCount); + EXPECT_EQ(1, SysCalls::lseekCalledCount); EXPECT_EQ(0llu, drmAllocation->getGpuBaseAddress()); @@ -2068,6 +2070,7 @@ TEST_F(DrmMemoryManagerTest, givenLimitedRangeAllocatorWhenBufferFromSharedHandl mock->ioctlExpected.primeFdToHandle = 1; mock->ioctlExpected.gemWait = 1; mock->ioctlExpected.gemClose = 1; + SysCalls::lseekCalledCount = 0; memoryManager->forceLimitedRangeAllocator(0xFFFFFFFFF); osHandle handle = 1u; @@ -2078,7 +2081,7 @@ TEST_F(DrmMemoryManagerTest, givenLimitedRangeAllocatorWhenBufferFromSharedHandl auto drmAllocation = static_cast(graphicsAllocation); EXPECT_EQ(0llu, drmAllocation->getGpuBaseAddress()); - EXPECT_EQ(1, lseekCalledCount); + EXPECT_EQ(1, SysCalls::lseekCalledCount); memoryManager->freeGraphicsMemory(graphicsAllocation); } @@ -2086,6 +2089,7 @@ TEST_F(DrmMemoryManagerTest, givenNon32BitAddressingWhenBufferFromSharedHandleIs mock->ioctlExpected.primeFdToHandle = 1; mock->ioctlExpected.gemWait = 1; mock->ioctlExpected.gemClose = 1; + SysCalls::lseekCalledCount = 0; memoryManager->setForce32BitAllocations(false); osHandle handle = 1u; @@ -2094,7 +2098,7 @@ TEST_F(DrmMemoryManagerTest, givenNon32BitAddressingWhenBufferFromSharedHandleIs auto graphicsAllocation = memoryManager->createGraphicsAllocationFromSharedHandle(handle, properties, true, false, true, nullptr); auto drmAllocation = static_cast(graphicsAllocation); EXPECT_FALSE(graphicsAllocation->is32BitAllocation()); - EXPECT_EQ(1, lseekCalledCount); + EXPECT_EQ(1, SysCalls::lseekCalledCount); EXPECT_EQ(0llu, drmAllocation->getGpuBaseAddress()); memoryManager->freeGraphicsMemory(graphicsAllocation); } @@ -2404,7 +2408,8 @@ TEST_F(DrmMemoryManagerTest, given32BitAllocatorWithHeapAllocatorWhenLargerFragm TEST_F(DrmMemoryManagerTest, givenSharedAllocationWithSmallerThenRealSizeWhenCreateIsCalledThenRealSizeIsUsed) { unsigned int realSize = 64 * 1024; - lseekReturn = realSize; + VariableBackup lseekBackup(&SysCalls::lseekReturn, realSize); + SysCalls::lseekCalledCount = 0; mock->ioctlExpected.primeFdToHandle = 1; mock->ioctlExpected.gemWait = 1; mock->ioctlExpected.gemClose = 1; @@ -2423,7 +2428,7 @@ TEST_F(DrmMemoryManagerTest, givenSharedAllocationWithSmallerThenRealSizeWhenCre EXPECT_NE(0llu, bo->peekAddress()); EXPECT_EQ(1u, bo->getRefCount()); EXPECT_EQ(realSize, bo->peekSize()); - EXPECT_EQ(1, lseekCalledCount); + EXPECT_EQ(1, SysCalls::lseekCalledCount); memoryManager->freeGraphicsMemory(graphicsAllocation); }