Skip to content

Commit

Permalink
Merge #2806
Browse files Browse the repository at this point in the history
2806: tests/ShmBacking: Improve invalid access tests. r=Saviq a=RAOF

So, `mmap` will provide a mapping *of pages*, which means its going to be a multiple of `PAGE_SIZE`, even if the backing file is not a-multiple-of-`PAGE_SIZE` big. It zero-pads the end up to the next page, *and accesses to this zero-initialised region do not fault*.

Now, it turns out I picked `4000` as the backing size, which is close enough to the `PAGE_SIZE` of `4096` on *most* kernels Ubuntu builds that doubling it puts you in a separate page, so these tests pass on all systems with a 4K `PAGE_SIZE`.

ppc64el has a 64K `PAGE_SIZE` 😠

Fix this by querying `sysconf(_SC_PAGE_SIZE)` and making our backing file *exactly* one page in size. Combined with `mmap` being specified to map on a page-aligned address (which is really the only thing it could do), that guarantees that *any* access outside the valid range is going to hit a different page, and hence fault.

Fixes: #2774

Co-authored-by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
  • Loading branch information
bors[bot] and RAOF committed Jan 23, 2023
2 parents 2ecda2e + 1ca9db1 commit d0c43ca
Showing 1 changed file with 24 additions and 16 deletions.
40 changes: 24 additions & 16 deletions tests/unit-tests/test_shm_backing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,16 @@ TEST(ShmBacking, read_from_invalid_memory_returns_0)
{
using namespace testing;

constexpr size_t const shm_size = 4000;
constexpr size_t const claimed_size = shm_size * 2; // Lie about our backing size
/* mmap maps ranges in multiples of PAGE_SIZE. If we want to test invalid accesses,
* we need to make sure our invalid access will try to touch an address *not* in the
* same page as our valid memory.
*
* To do this, make our valid region exactly 1 page large. mmap() will map it at
* a page-aligned address, so that means *any* access outside the valid region
* will hit a different page.
*/
size_t const shm_size = sysconf(_SC_PAGE_SIZE);
size_t const claimed_size = shm_size + 1; // Lie about our backing size
auto shm_fd = make_shm_fd(shm_size);
auto backing = mir::shm::rw_pool_from_fd(shm_fd, claimed_size);

Expand All @@ -293,8 +301,8 @@ TEST(ShmBacking, access_fault_is_true_after_invaild_read)
{
using namespace testing;

constexpr size_t const shm_size = 4000;
constexpr size_t const claimed_size = shm_size * 2; // Lie about our backing size
size_t const shm_size = sysconf(_SC_PAGE_SIZE);
size_t const claimed_size = shm_size + 1; // Lie about our backing size
auto shm_fd = make_shm_fd(shm_size);
auto backing = mir::shm::rw_pool_from_fd(shm_fd, claimed_size);

Expand All @@ -314,8 +322,8 @@ TEST(ShmBacking, access_fault_is_true_after_invaild_write)
{
using namespace testing;

constexpr size_t const shm_size = 4000;
constexpr size_t const claimed_size = shm_size * 2; // Lie about our backing size
size_t const shm_size = sysconf(_SC_PAGE_SIZE);
size_t const claimed_size = shm_size + 1; // Lie about our backing size
auto shm_fd = make_shm_fd(shm_size);
auto backing = mir::shm::rw_pool_from_fd(shm_fd, claimed_size);

Expand All @@ -332,8 +340,8 @@ TEST(ShmBacking, access_into_invalid_range_works_even_after_backing_destroyed)
{
using namespace testing;

constexpr size_t const shm_size = 4000;
constexpr size_t const claimed_size = shm_size * 2;
size_t const shm_size = sysconf(_SC_PAGE_SIZE);
size_t const claimed_size = shm_size + 1; // Lie about our backing size
auto shm_fd = make_shm_fd(shm_size);
auto backing = mir::shm::rw_pool_from_fd(shm_fd, claimed_size);

Expand Down Expand Up @@ -403,8 +411,8 @@ TEST(ShmBacking, can_resize_pool)
{
using namespace testing;

constexpr size_t const initial_size = 4000;
constexpr size_t const new_size = initial_size + 400;
size_t const initial_size = sysconf(_SC_PAGE_SIZE);
size_t const new_size = initial_size + 400;

auto shm_fd = make_shm_fd(initial_size);
auto backing = mir::shm::rw_pool_from_fd(shm_fd, initial_size);
Expand All @@ -431,8 +439,8 @@ TEST(ShmBacking, ranges_remain_valid_after_resize)
{
using namespace testing;

constexpr size_t const initial_size = 4000;
constexpr size_t const new_size = initial_size + 400;
size_t const initial_size = sysconf(_SC_PAGE_SIZE);
size_t const new_size = initial_size + 400;

auto shm_fd = make_shm_fd(initial_size);
auto backing = mir::shm::rw_pool_from_fd(shm_fd, initial_size);
Expand Down Expand Up @@ -463,8 +471,8 @@ TEST(ShmBacking, mapping_remains_valid_after_resize)
{
using namespace testing;

constexpr size_t const initial_size = 4000;
constexpr size_t const new_size = initial_size + 400;
size_t const initial_size = sysconf(_SC_PAGE_SIZE);
size_t const new_size = initial_size + 400;

auto shm_fd = make_shm_fd(initial_size);
auto backing = mir::shm::rw_pool_from_fd(shm_fd, initial_size);
Expand Down Expand Up @@ -494,7 +502,7 @@ TEST(ShmBacking, resize_rechecks_backing_size)
{
using namespace testing;

constexpr size_t const shm_size = 4000;
size_t const shm_size = sysconf(_SC_PAGE_SIZE);

mir::Fd shm_fd;
try
Expand Down Expand Up @@ -539,7 +547,7 @@ TEST(ShmBacking, resize_doesnt_install_sigbus_handler_when_safe)
{
using namespace testing;

constexpr size_t const shm_size = 4000;
size_t const shm_size = sysconf(_SC_PAGE_SIZE);

mir::Fd shm_fd;
try
Expand Down

0 comments on commit d0c43ca

Please sign in to comment.