RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-10-31 16:02:45 +00:00
Code Issues Activity Actions

Kernel: Use the Syscall string and buffer types more

Browse source 
While I was updating syscalls to stop passing null-terminated strings,
I added some helpful struct types:

    - StringArgument { const char*; size_t; }
    - ImmutableBuffer<Data, Size> { const Data*; Size; }
    - MutableBuffer<Data, Size> { Data*; Size; }

The Process class has some convenience functions for validating and
optionally extracting the contents from these structs:

    - get_syscall_path_argument(StringArgument)
    - validate_and_copy_string_from_user(StringArgument)
    - validate(ImmutableBuffer)
    - validate(MutableBuffer)

There's still so much code around this and I'm wondering if we should
generate most of it instead. Possible nice little project.
This commit is contained in:
Andreas Kling 2020-01-11 12:47:47 +01:00
parent 1434f30f92
commit 24c736b0e7
8 changed files with 148 additions and 121 deletions
Download patch file Download diff file Expand all files Collapse all files

163
Kernel/Process.cpp
View file

@ -218,9 +218,9 @@ int Process::sys$set_mmap_name(const Syscall::SC_set_mmap_name_params* user_para
Syscall::SC_set_mmap_name_params params;
copy_from_user(&params, user_params, sizeof(params));
if (!validate_read(params.name, params.name_length))
auto name = validate_and_copy_string_from_user(params.name);
if (name.is_null())
return -EFAULT;
auto name = copy_string_from_user(params.name, params.name_length);
auto* region = region_from_range({ VirtualAddress((u32)params.addr), params.size });
if (!region)
@ -294,9 +294,12 @@ void* Process::sys$mmap(const Syscall::SC_mmap_params* user_params)
int fd = params.fd;
int offset = params.offset;
if (params.name && !validate_read(params.name, params.name_length))
return (void*)-EFAULT;
auto name = copy_string_from_user(params.name, params.name_length);
String name;
if (params.name.characters) {
name = validate_and_copy_string_from_user(params.name);
if (name.is_null())
return (void*)-EFAULT;
}
if (size == 0)
return (void*)-EINVAL;
@ -922,10 +925,11 @@ int Process::sys$execve(const Syscall::SC_execve_params* user_params)
if (params.arguments.length > ARG_MAX || params.environment.length > ARG_MAX)
return -E2BIG;
if (!validate_read(params.path.characters, params.path.length))
auto path = validate_and_copy_string_from_user(params.path);
if (path.is_null())
return -EFAULT;
if (params.path.length == 0)
if (path.is_empty())
return -ENOENT;
auto copy_user_strings = [&](const auto& list, auto& output) {
@ -952,8 +956,6 @@ int Process::sys$execve(const Syscall::SC_execve_params* user_params)
if (!copy_user_strings(params.environment, environment))
return -EFAULT;
auto path = copy_string_from_user(params.path.characters, params.path.length);
int rc = exec(move(path), move(arguments), move(environment));
ASSERT(rc < 0); // We should never continue after a successful exec!
return rc;
@ -1448,7 +1450,7 @@ int Process::sys$close(int fd)
{
auto description = file_description(fd);
#ifdef DEBUG_IO
dbgprintf("%s(%u) sys$close(%d) %p\n", name().characters(), pid(), fd, description);
dbgprintf("%s(%u) sys$close(%d) %p\n", name().characters(), pid(), fd, description.ptr());
#endif
if (!description)
return -EBADF;
@ -1571,13 +1573,24 @@ bool Process::validate(const Syscall::MutableBufferArgument<DataType, SizeType>&
return validate_write(buffer.data, buffer.size);
}
String Process::validate_and_copy_string_from_user(const Syscall::StringArgument& string) const
template<typename DataType, typename SizeType>
bool Process::validate(const Syscall::ImmutableBufferArgument<DataType, SizeType>& buffer)
{
if (!validate_read(string.characters, string.length))
return validate_read(buffer.data, buffer.size);
}
String Process::validate_and_copy_string_from_user(const char* user_characters, size_t user_length) const
{
if (!validate_read(user_characters, user_length))
return {};
SmapDisabler disabler;
size_t length = strnlen(string.characters, string.length);
return String(string.characters, length);
size_t measured_length = strnlen(user_characters, user_length);
return String(user_characters, measured_length);
}
String Process::validate_and_copy_string_from_user(const Syscall::StringArgument& string) const
{
return validate_and_copy_string_from_user(string.characters, string.length);
}
int Process::sys$readlink(const Syscall::SC_readlink_params* user_params)
@ -1590,7 +1603,7 @@ int Process::sys$readlink(const Syscall::SC_readlink_params* user_params)
if (!validate(params.buffer))
return -EFAULT;
auto path = get_syscall_path_argument(params.path.characters, params.path.length);
auto path = get_syscall_path_argument(params.path);
if (path.is_error())
return path.error();
@ -1674,22 +1687,20 @@ int Process::sys$open(const Syscall::SC_open_params* user_params)
auto options = params.options;
auto mode = params.mode;
if (params.path_length <= 0)
return -EINVAL;
if (!validate_read(params.path, params.path_length))
return -EFAULT;
auto path = get_syscall_path_argument(params.path);
if (path.is_error())
return path.error();
// Ignore everything except permission bits.
mode &= 04777;
String path = copy_string_from_user(params.path, params.path_length);
int fd = alloc_fd();
#ifdef DEBUG_IO
dbgprintf("%s(%u) sys$open(\"%s\") -> %d\n", name().characters(), pid(), path, fd);
dbgprintf("%s(%u) sys$open(\"%s\") -> %d\n", name().characters(), pid(), path.value().characters(), fd);
#endif
if (fd < 0)
return fd;
auto result = VFS::the().open(path, options, mode & ~umask(), current_directory());
auto result = VFS::the().open(path.value(), options, mode & ~umask(), current_directory());
if (result.is_error())
return result.error();
auto description = result.value();
@ -1714,13 +1725,11 @@ int Process::sys$openat(const Syscall::SC_openat_params* user_params)
// Ignore everything except permission bits.
mode &= 04777;
if (params.path_length <= 0)
return -EINVAL;
if (!validate_read(params.path, params.path_length))
return -EFAULT;
auto path = copy_string_from_user(params.path, params.path_length);
auto path = get_syscall_path_argument(params.path);
if (path.is_error())
return path.error();
#ifdef DEBUG_IO
dbgprintf("%s(%u) sys$openat(%d, \"%s\")\n", dirfd, name().characters(), pid(), path);
dbgprintf("%s(%u) sys$openat(%d, \"%s\")\n", dirfd, name().characters(), pid(), path.value().characters());
#endif
int fd = alloc_fd();
if (fd < 0)
@ -1740,7 +1749,7 @@ int Process::sys$openat(const Syscall::SC_openat_params* user_params)
base = base_description->custody();
}
auto result = VFS::the().open(path, options, mode & ~umask(), *base);
auto result = VFS::the().open(path.value(), options, mode & ~umask(), *base);
if (result.is_error())
return result.error();
auto description = result.value();
@ -2365,10 +2374,10 @@ int Process::sys$realpath(const Syscall::SC_realpath_params* user_params)
Syscall::SC_realpath_params params;
copy_from_user(&params, user_params, sizeof(params));
if (!validate_write(params.buffer, params.buffer_size))
if (!validate_write(params.buffer.data, params.buffer.size))
return -EFAULT;
auto path = get_syscall_path_argument(params.path, params.path_length);
auto path = get_syscall_path_argument(params.path);
if (path.is_error())
return path.error();
@ -2384,10 +2393,10 @@ int Process::sys$realpath(const Syscall::SC_realpath_params* user_params)
ASSERT_NOT_REACHED();
}
if (canonical_path.string().length() + 1 > params.buffer_size)
if (canonical_path.string().length() + 1 > params.buffer.size)
return -ENAMETOOLONG;
copy_to_user(params.buffer, canonical_path.string().characters(), canonical_path.string().length() + 1);
copy_to_user(params.buffer.data, canonical_path.string().characters(), canonical_path.string().length() + 1);
return 0;
};
@ -2586,16 +2595,16 @@ int Process::sys$symlink(const Syscall::SC_symlink_params* user_params)
return -EFAULT;
Syscall::SC_symlink_params params;
copy_from_user(&params, user_params);
auto target = get_syscall_path_argument(params.target.characters, params.target.length);
auto target = get_syscall_path_argument(params.target);
if (target.is_error())
return target.error();
auto linkpath = get_syscall_path_argument(params.linkpath.characters, params.linkpath.length);
auto linkpath = get_syscall_path_argument(params.linkpath);
if (linkpath.is_error())
return linkpath.error();
return VFS::the().symlink(target.value(), linkpath.value(), current_directory());
}
KResultOr<String> Process::get_syscall_path_argument(const char* user_path, size_t path_length)
KResultOr<String> Process::get_syscall_path_argument(const char* user_path, size_t path_length) const
{
if (path_length == 0)
return KResult(-EINVAL);
@ -2606,6 +2615,11 @@ KResultOr<String> Process::get_syscall_path_argument(const char* user_path, size
return copy_string_from_user(user_path, path_length);
}
KResultOr<String> Process::get_syscall_path_argument(const Syscall::StringArgument& path) const
{
return get_syscall_path_argument(path.characters, path.length);
}
int Process::sys$rmdir(const char* user_path, size_t path_length)
{
auto path = get_syscall_path_argument(user_path, path_length);
@ -2644,7 +2658,7 @@ int Process::sys$chown(const Syscall::SC_chown_params* user_params)
return -EFAULT;
Syscall::SC_chown_params params;
copy_from_user(&params, user_params, sizeof(params));
auto path = get_syscall_path_argument(params.path.characters, params.path.length);
auto path = get_syscall_path_argument(params.path);
if (path.is_error())
return path.error();
return VFS::the().chown(path.value(), params.uid, params.gid, current_directory());
@ -2884,47 +2898,45 @@ int Process::sys$connect(int sockfd, const sockaddr* address, socklen_t address_
return socket.connect(*description, address, address_size, description->is_blocking() ? ShouldBlock::Yes : ShouldBlock::No);
}
ssize_t Process::sys$sendto(const Syscall::SC_sendto_params* params)
ssize_t Process::sys$sendto(const Syscall::SC_sendto_params* user_params)
{
if (!validate_read_typed(params))
if (!validate_read_typed(user_params))
return -EFAULT;
SmapDisabler disabler;
Syscall::SC_sendto_params params;
copy_from_user(&params, user_params);
int sockfd = params->sockfd;
const void* data = params->data;
size_t data_length = params->data_length;
int flags = params->flags;
const sockaddr* addr = params->addr;
socklen_t addr_length = params->addr_length;
int flags = params.flags;
const sockaddr* addr = params.addr;
socklen_t addr_length = params.addr_length;
if (!validate_read(data, data_length))
if (!validate(params.data))
return -EFAULT;
if (addr && !validate_read(addr, addr_length))
return -EFAULT;
auto description = file_description(sockfd);
auto description = file_description(params.sockfd);
if (!description)
return -EBADF;
if (!description->is_socket())
return -ENOTSOCK;
SmapDisabler disabler;
auto& socket = *description->socket();
return socket.sendto(*description, data, data_length, flags, addr, addr_length);
return socket.sendto(*description, params.data.data, params.data.size, flags, addr, addr_length);
}
ssize_t Process::sys$recvfrom(const Syscall::SC_recvfrom_params* params)
ssize_t Process::sys$recvfrom(const Syscall::SC_recvfrom_params* user_params)
{
if (!validate_read_typed(params))
if (!validate_read_typed(user_params))
return -EFAULT;
SmapDisabler disabler;
int sockfd = params->sockfd;
void* buffer = params->buffer;
size_t buffer_length = params->buffer_length;
int flags = params->flags;
sockaddr* addr = params->addr;
socklen_t* addr_length = params->addr_length;
Syscall::SC_recvfrom_params params;
copy_from_user(&params, user_params);
if (!validate_write(buffer, buffer_length))
int flags = params.flags;
sockaddr* addr = params.addr;
socklen_t* addr_length = params.addr_length;
if (!validate(params.buffer))
return -EFAULT;
if (addr_length) {
if (!validate_write_typed(addr_length))
@ -2934,7 +2946,7 @@ ssize_t Process::sys$recvfrom(const Syscall::SC_recvfrom_params* params)
} else if (addr) {
return -EINVAL;
}
auto description = file_description(sockfd);
auto description = file_description(params.sockfd);
if (!description)
return -EBADF;
if (!description->is_socket())
@ -2945,7 +2957,8 @@ ssize_t Process::sys$recvfrom(const Syscall::SC_recvfrom_params* params)
if (flags & MSG_DONTWAIT)
description->set_blocking(false);
auto nrecv = socket.recvfrom(*description, buffer, buffer_length, flags, addr, addr_length);
SmapDisabler disabler;
auto nrecv = socket.recvfrom(*description, params.buffer.data, params.buffer.size, flags, addr, addr_length);
if (flags & MSG_DONTWAIT)
description->set_blocking(original_blocking);
@ -3411,16 +3424,12 @@ int Process::sys$join_thread(int tid, void** exit_value)
return 0;
}
int Process::sys$set_thread_name(int tid, const char* buffer, int buffer_size)
int Process::sys$set_thread_name(int tid, const char* user_name, size_t user_name_length)
{
if (buffer_size < 0)
return -EINVAL;
if (!validate_read(buffer, buffer_size))
auto name = validate_and_copy_string_from_user(user_name, user_name_length);
if (name.is_null())
return -EFAULT;
auto name = copy_string_from_user(buffer, buffer_size);
const size_t max_thread_name_size = 64;
if (name.length() > max_thread_name_size)
return -EINVAL;
@ -3432,9 +3441,9 @@ int Process::sys$set_thread_name(int tid, const char* buffer, int buffer_size)
thread->set_name(name);
return 0;
}
int Process::sys$get_thread_name(int tid, char* buffer, int buffer_size)
int Process::sys$get_thread_name(int tid, char* buffer, size_t buffer_size)
{
if (buffer_size <= 0)
if (buffer_size == 0)
return -EINVAL;
if (!validate_write(buffer, buffer_size))
@ -3474,10 +3483,10 @@ int Process::sys$rename(const Syscall::SC_rename_params* user_params)
return -EFAULT;
Syscall::SC_rename_params params;
copy_from_user(&params, user_params);
auto old_path = get_syscall_path_argument(params.old_path.characters, params.old_path.length);
auto old_path = get_syscall_path_argument(params.old_path);
if (old_path.is_error())
return old_path.error();
auto new_path = get_syscall_path_argument(params.new_path.characters, params.new_path.length);
auto new_path = get_syscall_path_argument(params.new_path);
if (new_path.is_error())
return new_path.error();
return VFS::the().rename(old_path.value(), new_path.value(), current_directory());
@ -3685,7 +3694,7 @@ int Process::sys$mknod(const Syscall::SC_mknod_params* user_params)
copy_from_user(&params, user_params);
if (!is_superuser() && !is_regular_file(params.mode) && !is_fifo(params.mode) && !is_socket(params.mode))
return -EPERM;
auto path = get_syscall_path_argument(params.path.characters, params.path.length);
auto path = get_syscall_path_argument(params.path);
if (path.is_error())
return path.error();
return VFS::the().mknod(path.value(), params.mode & ~umask(), params.dev, current_directory());
@ -4002,14 +4011,14 @@ int Process::sys$module_load(const char* user_path, size_t path_length)
return 0;
}
int Process::sys$module_unload(const char* name, size_t name_length)
int Process::sys$module_unload(const char* user_name, size_t name_length)
{
if (!is_superuser())
return -EPERM;
if (!validate_read(name, name_length))
return -EFAULT;
auto module_name = copy_string_from_user(name, name_length);
auto module_name = validate_and_copy_string_from_user(user_name, name_length);
if (module_name.is_null())
return -EFAULT;
auto it = g_modules->find(module_name);
if (it == g_modules->end())

10
Kernel/Process.h
View file

@ -202,8 +202,8 @@ public:
void sys$exit_thread(void*);
int sys$join_thread(int tid, void** exit_value);
int sys$detach_thread(int tid);
int sys$set_thread_name(int tid, const char* buffer, int buffer_size);
int sys$get_thread_name(int tid, char* buffer, int buffer_size);
int sys$set_thread_name(int tid, const char* buffer, size_t buffer_size);
int sys$get_thread_name(int tid, char* buffer, size_t buffer_size);
int sys$rename(const Syscall::SC_rename_params*);
int sys$systrace(pid_t);
int sys$mknod(const Syscall::SC_mknod_params*);
@ -262,7 +262,10 @@ public:
bool validate_write_typed(T* value, size_t count = 1) { return validate_write(value, sizeof(T) * count); }
template<typename DataType, typename SizeType>
bool validate(const Syscall::MutableBufferArgument<DataType, SizeType>&);
template<typename DataType, typename SizeType>
bool validate(const Syscall::ImmutableBufferArgument<DataType, SizeType>&);
String validate_and_copy_string_from_user(const char*, size_t) const;
String validate_and_copy_string_from_user(const Syscall::StringArgument&) const;
Custody& current_directory();
@ -334,7 +337,8 @@ private:
KResult do_kill(Process&, int signal);
KResult do_killpg(pid_t pgrp, int signal);
KResultOr<String> get_syscall_path_argument(const char* user_path, size_t path_length);
KResultOr<String> get_syscall_path_argument(const char* user_path, size_t path_length) const;
KResultOr<String> get_syscall_path_argument(const Syscall::StringArgument&) const;
RefPtr<PageDirectory> m_page_directory;

66
Kernel/Syscall.h
View file

@ -184,6 +184,28 @@ inline constexpr const char* to_string(Function function)
}
#ifdef __serenity__
struct StringArgument {
const char* characters { nullptr };
size_t length { 0 };
};
template<typename DataType, typename SizeType>
struct MutableBufferArgument {
DataType* data { nullptr };
SizeType size { 0 };
};
template<typename DataType, typename SizeType>
struct ImmutableBufferArgument {
const DataType* data { nullptr };
SizeType size { 0 };
};
struct StringListArgument {
StringArgument* strings { nullptr };
size_t length { 0 };
};
struct SC_mmap_params {
uint32_t addr;
uint32_t size;
@ -191,21 +213,18 @@ struct SC_mmap_params {
int32_t flags;
int32_t fd;
int32_t offset; // FIXME: 64-bit off_t?
const char* name;
size_t name_length;
StringArgument name;
};
struct SC_open_params {
const char* path;
int path_length;
StringArgument path;
int options;
u16 mode;
};
struct SC_openat_params {
int dirfd;
const char* path;
int path_length;
StringArgument path;
int options;
u16 mode;
};
@ -227,8 +246,7 @@ struct SC_clock_nanosleep_params {
struct SC_sendto_params {
int sockfd;
const void* data;
size_t data_length;
ImmutableBufferArgument<void, size_t> data;
int flags;
const sockaddr* addr;
socklen_t addr_length;
@ -236,8 +254,7 @@ struct SC_sendto_params {
struct SC_recvfrom_params {
int sockfd;
void* buffer;
size_t buffer_length;
MutableBufferArgument<void, size_t> buffer;
int flags;
sockaddr* addr;
socklen_t* addr_length;
@ -288,39 +305,20 @@ struct SC_create_thread_params {
};
struct SC_realpath_params {
const char* path;
size_t path_length;
char* buffer;
size_t buffer_size;
StringArgument path;
MutableBufferArgument<char, size_t> buffer;
};
struct SC_set_mmap_name_params {
void* addr;
size_t size;
const char* name;
size_t name_length;
};
struct StringArgument {
const char* characters { nullptr };
size_t length { 0 };
};
template<typename DataType, typename SizeType>
struct MutableBufferArgument {
DataType* data { nullptr };
SizeType size { 0 };
};
struct SyscallStringList {
StringArgument* strings { nullptr };
size_t length { 0 };
StringArgument name;
};
struct SC_execve_params {
StringArgument path;
SyscallStringList arguments;
SyscallStringList environment;
StringListArgument arguments;
StringListArgument environment;
};
struct SC_readlink_params {

12
Libraries/LibC/fcntl.cpp
View file

@ -33,22 +33,30 @@ int creat_with_path_length(const char* path, size_t path_length, mode_t mode)
int open_with_path_length(const char* path, size_t path_length, int options, mode_t mode)
{
if (!path) {
errno = EFAULT;
return -1;
}
if (path_length > INT32_MAX) {
errno = EINVAL;
return -1;
}
Syscall::SC_open_params params { path, (int)path_length, options, mode };
Syscall::SC_open_params params { { path, path_length }, options, mode };
int rc = syscall(SC_open, &params);
__RETURN_WITH_ERRNO(rc, rc, -1);
}
int openat_with_path_length(int dirfd, const char* path, size_t path_length, int options, mode_t mode)
{
if (!path) {
errno = EFAULT;
return -1;
}
if (path_length > INT32_MAX) {
errno = EINVAL;
return -1;
}
Syscall::SC_openat_params params { dirfd, path, (int)path_length, options, mode };
Syscall::SC_openat_params params { dirfd, { path, path_length }, options, mode };
int rc = syscall(SC_openat, &params);
__RETURN_WITH_ERRNO(rc, rc, -1);
}

4
Libraries/LibC/mman.cpp
View file

@ -13,7 +13,7 @@ void* mmap(void* addr, size_t size, int prot, int flags, int fd, off_t offset)
void* mmap_with_name(void* addr, size_t size, int prot, int flags, int fd, off_t offset, const char* name)
{
Syscall::SC_mmap_params params { (u32)addr, size, prot, flags, fd, offset, name, name ? strlen(name) : 0 };
Syscall::SC_mmap_params params { (u32)addr, size, prot, flags, fd, offset, { name, name ? strlen(name) : 0 } };
int rc = syscall(SC_mmap, &params);
if (rc < 0 && -rc < EMAXERRNO) {
errno = -rc;
@ -40,7 +40,7 @@ int set_mmap_name(void* addr, size_t size, const char* name)
errno = EFAULT;
return -1;
}
Syscall::SC_set_mmap_name_params params { addr, size, name, strlen(name) };
Syscall::SC_set_mmap_name_params params { addr, size, { name, strlen(name) } };
int rc = syscall(SC_set_mmap_name, &params);
__RETURN_WITH_ERRNO(rc, rc, -1);
}

2
Libraries/LibC/stdlib.cpp
View file

@ -711,7 +711,7 @@ char* realpath(const char* pathname, char* buffer)
size_t size = PATH_MAX;
if (buffer == nullptr)
buffer = (char*)malloc(size);
Syscall::SC_realpath_params params { pathname, strlen(pathname), buffer, size };
Syscall::SC_realpath_params params { { pathname, strlen(pathname) }, { buffer, size } };
int rc = syscall(SC_realpath, &params);
if (rc < 0) {
errno = -rc;

4
Libraries/LibC/sys/socket.cpp
View file

@ -38,7 +38,7 @@ int connect(int sockfd, const sockaddr* addr, socklen_t addrlen)
ssize_t sendto(int sockfd, const void* data, size_t data_length, int flags, const struct sockaddr* addr, socklen_t addr_length)
{
Syscall::SC_sendto_params params { sockfd, data, data_length, flags, addr, addr_length };
Syscall::SC_sendto_params params { sockfd, { data, data_length }, flags, addr, addr_length };
int rc = syscall(SC_sendto, &params);
__RETURN_WITH_ERRNO(rc, rc, -1);
}
@ -50,7 +50,7 @@ ssize_t send(int sockfd, const void* data, size_t data_length, int flags)
ssize_t recvfrom(int sockfd, void* buffer, size_t buffer_length, int flags, struct sockaddr* addr, socklen_t* addr_length)
{
Syscall::SC_recvfrom_params params { sockfd, buffer, buffer_length, flags, addr, addr_length };
Syscall::SC_recvfrom_params params { sockfd, { buffer, buffer_length }, flags, addr, addr_length };
int rc = syscall(SC_recvfrom, &params);
__RETURN_WITH_ERRNO(rc, rc, -1);
}

8
Libraries/LibPthread/pthread.cpp
View file

@ -539,11 +539,19 @@ int pthread_setspecific(pthread_key_t key, const void* value)
}
int pthread_setname_np(pthread_t thread, const char* buffer, int buffer_size)
{
if (buffer_size < 0) {
errno = EINVAL;
return -1;
}
return syscall(SC_set_thread_name, thread, buffer, buffer_size);
}
int pthread_getname_np(pthread_t thread, char* buffer, int buffer_size)
{
if (buffer_size < 0) {
errno = EINVAL;
return -1;
}
return syscall(SC_get_thread_name, thread, buffer, buffer_size);
}