Skip to content

Commit a0985cf

Browse files
xzpeterakpm00
authored andcommitted
mm/selftests: add a test to verify mmap_changing race with -EAGAIN
Add an unit test to verify the recent mmap_changing ABI breakage. Note that I used some tricks here and there to make the test simple, e.g. I abused UFFDIO_MOVE on top of shmem with the fact that I know what I want to test will be even earlier than the vma type check. Rich comments were added to explain trivial details. Before that fix, -EAGAIN would have been written to the copy field most of the time but not always; the test should be able to reliably trigger the outlier case. After the fix, it's written always, the test verifies that making sure corresponding field (e.g. copy.copy for UFFDIO_COPY) is updated. Link: https://lkml.kernel.org/r/[email protected] Signed-off-by: Peter Xu <[email protected]> Cc: Andrea Arcangeli <[email protected]> Cc: Axel Rasmussen <[email protected]> Cc: Mike Rapoport <[email protected]> Cc: Suren Baghdasaryan <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
1 parent 5aa0fb6 commit a0985cf

File tree

1 file changed

+203
-0
lines changed

1 file changed

+203
-0
lines changed

tools/testing/selftests/mm/uffd-unit-tests.c

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,6 +1231,183 @@ static void uffd_move_pmd_split_test(uffd_test_args_t *targs)
12311231
uffd_move_pmd_handle_fault);
12321232
}
12331233

1234+
static bool
1235+
uffdio_verify_results(const char *name, int ret, int error, long result)
1236+
{
1237+
/*
1238+
* Should always return -1 with errno=EAGAIN, with corresponding
1239+
* result field updated in ioctl() args to be -EAGAIN too
1240+
* (e.g. copy.copy field for UFFDIO_COPY).
1241+
*/
1242+
if (ret != -1) {
1243+
uffd_test_fail("%s should have returned -1", name);
1244+
return false;
1245+
}
1246+
1247+
if (error != EAGAIN) {
1248+
uffd_test_fail("%s should have errno==EAGAIN", name);
1249+
return false;
1250+
}
1251+
1252+
if (result != -EAGAIN) {
1253+
uffd_test_fail("%s should have been updated for -EAGAIN",
1254+
name);
1255+
return false;
1256+
}
1257+
1258+
return true;
1259+
}
1260+
1261+
/*
1262+
* This defines a function to test one ioctl. Note that here "field" can
1263+
* be 1 or anything not -EAGAIN. With that initial value set, we can
1264+
* verify later that it should be updated by kernel (when -EAGAIN
1265+
* returned), by checking whether it is also updated to -EAGAIN.
1266+
*/
1267+
#define DEFINE_MMAP_CHANGING_TEST(name, ioctl_name, field) \
1268+
static bool uffdio_mmap_changing_test_##name(int fd) \
1269+
{ \
1270+
int ret; \
1271+
struct uffdio_##name args = { \
1272+
.field = 1, \
1273+
}; \
1274+
ret = ioctl(fd, ioctl_name, &args); \
1275+
return uffdio_verify_results(#ioctl_name, ret, errno, args.field); \
1276+
}
1277+
1278+
DEFINE_MMAP_CHANGING_TEST(zeropage, UFFDIO_ZEROPAGE, zeropage)
1279+
DEFINE_MMAP_CHANGING_TEST(copy, UFFDIO_COPY, copy)
1280+
DEFINE_MMAP_CHANGING_TEST(move, UFFDIO_MOVE, move)
1281+
DEFINE_MMAP_CHANGING_TEST(poison, UFFDIO_POISON, updated)
1282+
DEFINE_MMAP_CHANGING_TEST(continue, UFFDIO_CONTINUE, mapped)
1283+
1284+
typedef enum {
1285+
/* We actually do not care about any state except UNINTERRUPTIBLE.. */
1286+
THR_STATE_UNKNOWN = 0,
1287+
THR_STATE_UNINTERRUPTIBLE,
1288+
} thread_state;
1289+
1290+
static void sleep_short(void)
1291+
{
1292+
usleep(1000);
1293+
}
1294+
1295+
static thread_state thread_state_get(pid_t tid)
1296+
{
1297+
const char *header = "State:\t";
1298+
char tmp[256], *p, c;
1299+
FILE *fp;
1300+
1301+
snprintf(tmp, sizeof(tmp), "/proc/%d/status", tid);
1302+
fp = fopen(tmp, "r");
1303+
1304+
if (!fp) {
1305+
return THR_STATE_UNKNOWN;
1306+
}
1307+
1308+
while (fgets(tmp, sizeof(tmp), fp)) {
1309+
p = strstr(tmp, header);
1310+
if (p) {
1311+
/* For example, "State:\tD (disk sleep)" */
1312+
c = *(p + sizeof(header) - 1);
1313+
return c == 'D' ?
1314+
THR_STATE_UNINTERRUPTIBLE : THR_STATE_UNKNOWN;
1315+
}
1316+
}
1317+
1318+
return THR_STATE_UNKNOWN;
1319+
}
1320+
1321+
static void thread_state_until(pid_t tid, thread_state state)
1322+
{
1323+
thread_state s;
1324+
1325+
do {
1326+
s = thread_state_get(tid);
1327+
sleep_short();
1328+
} while (s != state);
1329+
}
1330+
1331+
static void *uffd_mmap_changing_thread(void *opaque)
1332+
{
1333+
volatile pid_t *pid = opaque;
1334+
int ret;
1335+
1336+
/* Unfortunately, it's only fetch-able from the thread itself.. */
1337+
assert(*pid == 0);
1338+
*pid = syscall(SYS_gettid);
1339+
1340+
/* Inject an event, this will hang solid until the event read */
1341+
ret = madvise(area_dst, page_size, MADV_REMOVE);
1342+
if (ret)
1343+
err("madvise(MADV_REMOVE) failed");
1344+
1345+
return NULL;
1346+
}
1347+
1348+
static void uffd_consume_message(int fd)
1349+
{
1350+
struct uffd_msg msg = { 0 };
1351+
1352+
while (uffd_read_msg(fd, &msg));
1353+
}
1354+
1355+
static void uffd_mmap_changing_test(uffd_test_args_t *targs)
1356+
{
1357+
/*
1358+
* This stores the real PID (which can be different from how tid is
1359+
* defined..) for the child thread, 0 means not initialized.
1360+
*/
1361+
pid_t pid = 0;
1362+
pthread_t tid;
1363+
int ret;
1364+
1365+
if (uffd_register(uffd, area_dst, nr_pages * page_size,
1366+
true, false, false))
1367+
err("uffd_register() failed");
1368+
1369+
/* Create a thread to generate the racy event */
1370+
ret = pthread_create(&tid, NULL, uffd_mmap_changing_thread, &pid);
1371+
if (ret)
1372+
err("pthread_create() failed");
1373+
1374+
/*
1375+
* Wait until the thread setup the pid. Use volatile to make sure
1376+
* it reads from RAM not regs.
1377+
*/
1378+
while (!(volatile pid_t)pid)
1379+
sleep_short();
1380+
1381+
/* Wait until the thread hangs at REMOVE event */
1382+
thread_state_until(pid, THR_STATE_UNINTERRUPTIBLE);
1383+
1384+
if (!uffdio_mmap_changing_test_copy(uffd))
1385+
return;
1386+
1387+
if (!uffdio_mmap_changing_test_zeropage(uffd))
1388+
return;
1389+
1390+
if (!uffdio_mmap_changing_test_move(uffd))
1391+
return;
1392+
1393+
if (!uffdio_mmap_changing_test_poison(uffd))
1394+
return;
1395+
1396+
if (!uffdio_mmap_changing_test_continue(uffd))
1397+
return;
1398+
1399+
/*
1400+
* All succeeded above! Recycle everything. Start by reading the
1401+
* event so as to kick the thread roll again..
1402+
*/
1403+
uffd_consume_message(uffd);
1404+
1405+
ret = pthread_join(tid, NULL);
1406+
assert(ret == 0);
1407+
1408+
uffd_test_pass();
1409+
}
1410+
12341411
static int prevent_hugepages(const char **errmsg)
12351412
{
12361413
/* This should be done before source area is populated */
@@ -1470,6 +1647,32 @@ uffd_test_case_t uffd_tests[] = {
14701647
.mem_targets = MEM_ALL,
14711648
.uffd_feature_required = UFFD_FEATURE_POISON,
14721649
},
1650+
{
1651+
.name = "mmap-changing",
1652+
.uffd_fn = uffd_mmap_changing_test,
1653+
/*
1654+
* There's no point running this test over all mem types as
1655+
* they share the same code paths.
1656+
*
1657+
* Choose shmem for simplicity, because (1) shmem supports
1658+
* MINOR mode to cover UFFDIO_CONTINUE, and (2) shmem is
1659+
* almost always available (unlike hugetlb). Here we
1660+
* abused SHMEM for UFFDIO_MOVE, but the test we want to
1661+
* cover doesn't yet need the correct memory type..
1662+
*/
1663+
.mem_targets = MEM_SHMEM,
1664+
/*
1665+
* Any UFFD_FEATURE_EVENT_* should work to trigger the
1666+
* race logically, but choose the simplest (REMOVE).
1667+
*
1668+
* Meanwhile, since we'll cover quite a few new ioctl()s
1669+
* (CONTINUE, POISON, MOVE), skip this test for old kernels
1670+
* by choosing all of them.
1671+
*/
1672+
.uffd_feature_required = UFFD_FEATURE_EVENT_REMOVE |
1673+
UFFD_FEATURE_MOVE | UFFD_FEATURE_POISON |
1674+
UFFD_FEATURE_MINOR_SHMEM,
1675+
},
14731676
};
14741677

14751678
static void usage(const char *prog)

0 commit comments

Comments
 (0)