Skip to content

Commit 292b300

Browse files
gchateletgchatelet
authored
[libc][bug] Fix out of bound write in memcpy w/ software prefetching (#90591)
This patch adds tests for `memcpy` and `memset` making sure that we don't access buffers out of bounds. It relies on POSIX `mmap` / `mprotect` and works only when FULL_BUILD_MODE is disabled. The bug showed up while enabling software prefetching. `loop_and_tail_offset` is always running at least one iteration but in some configurations loop unrolled prefetching was actually needing only the tail operation and no loop iterations at all.
1 parent b1e99a6 commit 292b300

File tree

4 files changed

+185
-2
lines changed

4 files changed

+185
-2
lines changed

‎libc/src/string/memory_utils/x86_64/inline_memcpy.h

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,13 @@ inline_memcpy_x86_sse2_ge64_sw_prefetching(Ptr __restrict dst,
107107
offset += K_THREE_CACHELINES;
108108
}
109109
}
110-
return builtin::Memcpy<32>::loop_and_tail_offset(dst, src, count, offset);
110+
// We don't use 'loop_and_tail_offset' because it assumes at least one
111+
// iteration of the loop.
112+
while (offset + 32 <= count) {
113+
builtin::Memcpy<32>::block_offset(dst, src, offset);
114+
offset += 32;
115+
}
116+
return builtin::Memcpy<32>::tail(dst, src, count);
111117
}
112118

113119
[[maybe_unused]] LIBC_INLINE void
@@ -139,7 +145,13 @@ inline_memcpy_x86_avx_ge64_sw_prefetching(Ptr __restrict dst,
139145
builtin::Memcpy<K_THREE_CACHELINES>::block_offset(dst, src, offset);
140146
offset += K_THREE_CACHELINES;
141147
}
142-
return builtin::Memcpy<64>::loop_and_tail_offset(dst, src, count, offset);
148+
// We don't use 'loop_and_tail_offset' because it assumes at least one
149+
// iteration of the loop.
150+
while (offset + 64 <= count) {
151+
builtin::Memcpy<64>::block_offset(dst, src, offset);
152+
offset += 64;
153+
}
154+
return builtin::Memcpy<64>::tail(dst, src, count);
143155
}
144156

145157
[[maybe_unused]] LIBC_INLINE void

‎libc/test/src/string/memcpy_test.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,14 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "memory_utils/memory_check_utils.h"
10+
#include "src/__support/macros/properties/os.h" // LIBC_TARGET_OS_IS_LINUX
1011
#include "src/string/memcpy.h"
1112
#include "test/UnitTest/Test.h"
1213

14+
#if !defined(LIBC_FULL_BUILD) && defined(LIBC_TARGET_OS_IS_LINUX)
15+
#include "memory_utils/protected_pages.h"
16+
#endif // !defined(LIBC_FULL_BUILD) && defined(LIBC_TARGET_OS_IS_LINUX)
17+
1318
namespace LIBC_NAMESPACE {
1419

1520
// Adapt CheckMemcpy signature to memcpy.
@@ -30,4 +35,40 @@ TEST(LlvmLibcMemcpyTest, SizeSweep) {
3035
}
3136
}
3237

38+
#if !defined(LIBC_FULL_BUILD) && defined(LIBC_TARGET_OS_IS_LINUX)
39+
40+
TEST(LlvmLibcMemcpyTest, CheckAccess) {
41+
static constexpr size_t MAX_SIZE = 1024;
42+
LIBC_ASSERT(MAX_SIZE < GetPageSize());
43+
ProtectedPages pages;
44+
const Page write_buffer = pages.GetPageA().WithAccess(PROT_WRITE);
45+
const Page read_buffer = [&]() {
46+
// We fetch page B in write mode.
47+
auto page = pages.GetPageB().WithAccess(PROT_WRITE);
48+
// And fill it with random numbers.
49+
for (size_t i = 0; i < page.page_size; ++i)
50+
page.page_ptr[i] = rand();
51+
// Then return it in read mode.
52+
return page.WithAccess(PROT_READ);
53+
}();
54+
for (size_t size = 0; size < MAX_SIZE; ++size) {
55+
// We cross-check the function with two sources and two destinations.
56+
// - The first of them (bottom) is always page aligned and faults when
57+
// accessing bytes before it.
58+
// - The second one (top) is not necessarily aligned and faults when
59+
// accessing bytes after it.
60+
const uint8_t *sources[2] = {read_buffer.bottom(size),
61+
read_buffer.top(size)};
62+
uint8_t *destinations[2] = {write_buffer.bottom(size),
63+
write_buffer.top(size)};
64+
for (const uint8_t *src : sources) {
65+
for (uint8_t *dst : destinations) {
66+
LIBC_NAMESPACE::memcpy(dst, src, size);
67+
}
68+
}
69+
}
70+
}
71+
72+
#endif // !defined(LIBC_FULL_BUILD) && defined(LIBC_TARGET_OS_IS_LINUX)
73+
3374
} // namespace LIBC_NAMESPACE

‎libc/test/src/string/memory_utils/protected_pages.h

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
//===-- protected_pages.h -------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
// This file provides protected pages that fault when accessing prior or past
9+
// it. This is useful to check memory functions that must not access outside of
10+
// the provided size limited buffer.
11+
//===----------------------------------------------------------------------===//
12+
13+
#ifndef LIBC_TEST_SRC_STRING_MEMORY_UTILS_PROTECTED_PAGES_H
14+
#define LIBC_TEST_SRC_STRING_MEMORY_UTILS_PROTECTED_PAGES_H
15+
16+
#include "src/__support/macros/properties/os.h" // LIBC_TARGET_OS_IS_LINUX
17+
#if defined(LIBC_FULL_BUILD) || !defined(LIBC_TARGET_OS_IS_LINUX)
18+
#error "Protected pages requires mmap and cannot be used in full build mode."
19+
#endif // defined(LIBC_FULL_BUILD) || !defined(LIBC_TARGET_OS_IS_LINUX)
20+
21+
#include "src/__support/macros/attributes.h" // LIBC_INLINE
22+
#include <stddef.h> // size_t
23+
#include <stdint.h> // uint8_t
24+
#include <sys/mman.h> // mmap, munmap
25+
#include <unistd.h> // sysconf, _SC_PAGESIZE
26+
27+
// Returns mmap page size.
28+
LIBC_INLINE size_t GetPageSize() {
29+
static const size_t PAGE_SIZE = sysconf(_SC_PAGESIZE);
30+
return PAGE_SIZE;
31+
}
32+
33+
// Represents a page of memory whose access can be configured throught the
34+
// 'WithAccess' function. Accessing data above or below this page will trap as
35+
// it is sandwiched between two pages with no read / write access.
36+
struct Page {
37+
// Returns an aligned pointer that can be accessed up to page_size. Accessing
38+
// data at ptr[-1] will fault.
39+
LIBC_INLINE uint8_t *bottom(size_t size) const {
40+
if (size >= page_size)
41+
__builtin_trap();
42+
return page_ptr;
43+
}
44+
// Returns a pointer to a buffer that can be accessed up to size. Accessing
45+
// data at ptr[size] will trap.
46+
LIBC_INLINE uint8_t *top(size_t size) const {
47+
return page_ptr + page_size - size;
48+
}
49+
50+
// protection is one of PROT_READ / PROT_WRITE.
51+
LIBC_INLINE Page &WithAccess(int protection) {
52+
if (mprotect(page_ptr, page_size, protection) != 0)
53+
__builtin_trap();
54+
return *this;
55+
}
56+
57+
const size_t page_size;
58+
uint8_t *const page_ptr;
59+
};
60+
61+
// Allocates 5 consecutive pages that will trap if accessed.
62+
// | page layout | access | page name |
63+
// |-------------|--------|:---------:|
64+
// | 0 | trap | |
65+
// | 1 | custom | A |
66+
// | 2 | trap | |
67+
// | 3 | custom | B |
68+
// | 4 | trap | |
69+
//
70+
// The pages A and B can be retrieved as with 'GetPageA' / 'GetPageB' and their
71+
// accesses can be customized through the 'WithAccess' function.
72+
struct ProtectedPages {
73+
static constexpr size_t PAGES = 5;
74+
75+
ProtectedPages()
76+
: page_size(GetPageSize()),
77+
ptr(mmap(/*address*/ nullptr, /*length*/ PAGES * page_size,
78+
/*protection*/ PROT_NONE,
79+
/*flags*/ MAP_PRIVATE | MAP_ANONYMOUS, /*fd*/ -1,
80+
/*offset*/ 0)) {
81+
if (reinterpret_cast<intptr_t>(ptr) == -1)
82+
__builtin_trap();
83+
}
84+
~ProtectedPages() { munmap(ptr, PAGES * page_size); }
85+
86+
LIBC_INLINE Page GetPageA() const { return Page{page_size, page<1>()}; }
87+
LIBC_INLINE Page GetPageB() const { return Page{page_size, page<3>()}; }
88+
89+
private:
90+
template <size_t index> LIBC_INLINE uint8_t *page() const {
91+
static_assert(index < PAGES);
92+
return static_cast<uint8_t *>(ptr) + (index * page_size);
93+
}
94+
95+
const size_t page_size;
96+
void *const ptr = nullptr;
97+
};
98+
99+
#endif // LIBC_TEST_SRC_STRING_MEMORY_UTILS_PROTECTED_PAGES_H

‎libc/test/src/string/memset_test.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,14 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "memory_utils/memory_check_utils.h"
10+
#include "src/__support/macros/properties/os.h" // LIBC_TARGET_OS_IS_LINUX
1011
#include "src/string/memset.h"
1112
#include "test/UnitTest/Test.h"
1213

14+
#if !defined(LIBC_FULL_BUILD) && defined(LIBC_TARGET_OS_IS_LINUX)
15+
#include "memory_utils/protected_pages.h"
16+
#endif // !defined(LIBC_FULL_BUILD) && defined(LIBC_TARGET_OS_IS_LINUX)
17+
1318
namespace LIBC_NAMESPACE {
1419

1520
// Adapt CheckMemset signature to memset.
@@ -27,4 +32,30 @@ TEST(LlvmLibcMemsetTest, SizeSweep) {
2732
}
2833
}
2934

35+
#if !defined(LIBC_FULL_BUILD) && defined(LIBC_TARGET_OS_IS_LINUX)
36+
37+
TEST(LlvmLibcMemsetTest, CheckAccess) {
38+
static constexpr size_t MAX_SIZE = 1024;
39+
LIBC_ASSERT(MAX_SIZE < GetPageSize());
40+
ProtectedPages pages;
41+
const Page write_buffer = pages.GetPageA().WithAccess(PROT_WRITE);
42+
const cpp::array<int, 2> fill_chars = {0, 0x7F};
43+
for (int fill_char : fill_chars) {
44+
for (size_t size = 0; size < MAX_SIZE; ++size) {
45+
// We cross-check the function with two destinations.
46+
// - The first of them (bottom) is always page aligned and faults when
47+
// accessing bytes before it.
48+
// - The second one (top) is not necessarily aligned and faults when
49+
// accessing bytes after it.
50+
uint8_t *destinations[2] = {write_buffer.bottom(size),
51+
write_buffer.top(size)};
52+
for (uint8_t *dst : destinations) {
53+
LIBC_NAMESPACE::memset(dst, fill_char, size);
54+
}
55+
}
56+
}
57+
}
58+
59+
#endif // !defined(LIBC_FULL_BUILD) && defined(LIBC_TARGET_OS_IS_LINUX)
60+
3061
} // namespace LIBC_NAMESPACE

0 commit comments

Comments
 (0)