Skip to content

Commit

Permalink
Kernel: Don't ignore validation result in ptrace(PT_PEEK)
Browse files Browse the repository at this point in the history
Also mark all of the address validation functions [[nodiscard]] to turn
this kind of bug into a compile error in the future.
  • Loading branch information
awesomekling committed Apr 13, 2020
1 parent e432a27 commit c8edcf1
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 11 deletions.
20 changes: 10 additions & 10 deletions Kernel/Process.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,14 +326,14 @@ class Process : public InlineLinkedListNode<Process> {
u32 m_ticks_in_user_for_dead_children { 0 };
u32 m_ticks_in_kernel_for_dead_children { 0 };

bool validate_read_from_kernel(VirtualAddress, size_t) const;
[[nodiscard]] bool validate_read_from_kernel(VirtualAddress, size_t) const;

bool validate_read(const void*, size_t) const;
bool validate_write(void*, size_t) const;
[[nodiscard]] bool validate_read(const void*, size_t) const;
[[nodiscard]] bool validate_write(void*, size_t) const;
template<typename T>
bool validate_read_typed(T* value, size_t count = 1) { return validate_read(value, sizeof(T) * count); }
[[nodiscard]] bool validate_read_typed(T* value, size_t count = 1) { return validate_read(value, sizeof(T) * count); }
template<typename T>
bool validate_read_and_copy_typed(T* dest, const T* src)
[[nodiscard]] bool validate_read_and_copy_typed(T* dest, const T* src)
{
bool validated = validate_read_typed(src);
if (validated) {
Expand All @@ -342,14 +342,14 @@ class Process : public InlineLinkedListNode<Process> {
return validated;
}
template<typename T>
bool validate_write_typed(T* value, size_t count = 1) { return validate_write(value, sizeof(T) * count); }
[[nodiscard]] 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>&);
[[nodiscard]] bool validate(const Syscall::MutableBufferArgument<DataType, SizeType>&);
template<typename DataType, typename SizeType>
bool validate(const Syscall::ImmutableBufferArgument<DataType, SizeType>&);
[[nodiscard]] 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;
[[nodiscard]] String validate_and_copy_string_from_user(const char*, size_t) const;
[[nodiscard]] String validate_and_copy_string_from_user(const Syscall::StringArgument&) const;

Custody& current_directory();
Custody* executable() { return m_executable.ptr(); }
Expand Down
3 changes: 2 additions & 1 deletion Kernel/Ptrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ KResultOr<u32> handle_syscall(const Kernel::Syscall::SC_ptrace_params& params, P
auto result = peer->process().peek_user_data(peek_params.address);
if (result.is_error())
return -EFAULT;
peer->process().validate_write(peek_params.out_data, sizeof(u32));
if (!peer->process().validate_write(peek_params.out_data, sizeof(u32)))
return -EFAULT;
copy_from_user(peek_params.out_data, &result.value());
break;
}
Expand Down

0 comments on commit c8edcf1

Please sign in to comment.