From 7ed002d1ca52fed6e31ad50f5dff471dcb62be5f Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Fri, 15 Jan 2021 21:35:46 +0100 Subject: [PATCH] UserlandEmulator: Fix data loss in realpath/readlink This 'data loss' was introduced in 809a8ee6936c6e1f688b0854fa4214619d75f906, because I hoped we could eventually outlaw overlong paths entirely. This sparked some discussion: https://github.com/SerenityOS/serenity/discussions/4357 Among other things, we agree that yeah, the Kernel can and should be able to return paths of arbitrary length. This means that the 'arbitrary' maximum of PATH_MAX in UserspaceEmulator should be considered to be unnecessary data loss, and as such, needs to be fixed. --- .../DevTools/UserspaceEmulator/Emulator.cpp | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/Userland/DevTools/UserspaceEmulator/Emulator.cpp b/Userland/DevTools/UserspaceEmulator/Emulator.cpp index db73c53041..c43b47cd0b 100644 --- a/Userland/DevTools/UserspaceEmulator/Emulator.cpp +++ b/Userland/DevTools/UserspaceEmulator/Emulator.cpp @@ -1364,20 +1364,16 @@ int Emulator::virt$realpath(FlatPtr params_addr) Syscall::SC_realpath_params params; mmu().copy_from_vm(¶ms, params_addr, sizeof(params)); - if (params.path.length > PATH_MAX) { - return -ENAMETOOLONG; - } auto path = mmu().copy_buffer_from_vm((FlatPtr)params.path.characters, params.path.length); - char host_buffer[PATH_MAX] = {}; - size_t host_buffer_size = min(sizeof(host_buffer), params.buffer.size); + auto host_buffer = ByteBuffer::create_zeroed(params.buffer.size); Syscall::SC_realpath_params host_params; host_params.path = { (const char*)path.data(), path.size() }; - host_params.buffer = { host_buffer, host_buffer_size }; + host_params.buffer = { (char*)host_buffer.data(), host_buffer.size() }; int rc = syscall(SC_realpath, &host_params); if (rc < 0) return rc; - mmu().copy_to_vm((FlatPtr)params.buffer.data, host_buffer, host_buffer_size); + mmu().copy_to_vm((FlatPtr)params.buffer.data, host_buffer.data(), host_buffer.size()); return rc; } @@ -1759,20 +1755,16 @@ int Emulator::virt$readlink(FlatPtr params_addr) Syscall::SC_readlink_params params; mmu().copy_from_vm(¶ms, params_addr, sizeof(params)); - if (params.path.length > PATH_MAX) { - return -ENAMETOOLONG; - } auto path = mmu().copy_buffer_from_vm((FlatPtr)params.path.characters, params.path.length); - char host_buffer[PATH_MAX] = {}; - size_t host_buffer_size = min(sizeof(host_buffer), params.buffer.size); + auto host_buffer = ByteBuffer::create_zeroed(params.buffer.size); Syscall::SC_readlink_params host_params; host_params.path = { (const char*)path.data(), path.size() }; - host_params.buffer = { host_buffer, host_buffer_size }; + host_params.buffer = { (char*)host_buffer.data(), host_buffer.size() }; int rc = syscall(SC_readlink, &host_params); if (rc < 0) return rc; - mmu().copy_to_vm((FlatPtr)params.buffer.data, host_buffer, host_buffer_size); + mmu().copy_to_vm((FlatPtr)params.buffer.data, host_buffer.data(), host_buffer.size()); return rc; }