Sanity test suite #71

pakmarkthub · 2019-07-18T00:47:22Z

This PR depends on PR #60. DO NOT merge it before merging PR #60.

This change:

introduces the sanity test suite, which does basic functionality and invalidation tests.
consolidates "basic" and "validate" tests into "sanity" test.
creates test cases to ensure no security holes due to invalidation issues.

drossetti · 2019-07-30T11:33:54Z

@pakmarkthub now that the other PR of yours has been merged into the upstream invalidate branch, could you please rebase your dev-sanity-unittest tree?

…ted invalidation_one_process test case

…not interfere with each other

…_invalidation

…nix socket does not work

…lidation_fork_access_after_cumemfree

…ible with libcheck from version 0.9.8

…ssociated resources

…" clean up to gdrdrv, and added *.swp to .gitignore

… "C" to gdrapi_internal.h

pakmarkthub · 2019-07-30T23:25:28Z

@drossetti Rebased. Please take a look.

e-ago · 2019-08-01T17:36:09Z

I've executed the ./sanity -v test on a DGX-1V with:

Ubuntu 18.04, kernel 4.4
GPU V100, CUDA 10.1, r418.67

Few comments:

You should specify in the README that the check framework is required on the machine
dmesg returns some messages when using sockets in tests:

[Aug 1 10:23] gdrdrv:invoking unpin_buffer while callback has already been fired
[  +0.000006] gdrdrv:invoking unpin_buffer while callback has already been fired
[  +1.815832] gdrdrv:filp is not opened by the current process
[  +2.542915] gdrdrv:filp is not opened by the current process
[  +1.639312] gdrdrv:invoking unpin_buffer while callback has already been fired
[  +1.715075] gdrdrv:filp is not opened by the current process
[  +0.454921] gdrdrv:filp is not opened by the current process
[  +1.252773] gdrdrv:filp is not opened by the current process
[  +0.078880] gdrdrv:filp is not opened by the current process

I still have to go through the entire code

pakmarkthub · 2019-08-01T17:45:39Z

Thank you @e-ago. dmesg is a good catch. I should use gdr_dbg instead of gdr_err to suppress the message. By the way, this is the expected behavior because "sanity" verifies that Process A cannot spy on Process B via gdrcopy.

…om gdr_err to gdr_dbg

pakmarkthub · 2019-08-06T21:16:33Z

@e-ago I have fixed the code according to your comments. Please take a look.

e-ago · 2019-08-05T10:44:24Z

sanity.cpp

+    memset(copy_buf, 0xA5, size * sizeof(*copy_buf));
+
+    print_dbg("check 2: gdr_copy_to_bar() + read back via cuMemcpy D->H\n");
+    gdr_copy_to_bar(buf_ptr, init_buf, size);


In check 1 test you already set the value of buf_ptr with the same values ininit_buf. Maybe you can cleanup buf_ptr before copying again init_buf values with a different method (as you do for copy_buf)

e-ago · 2019-08-05T10:49:21Z

sanity.cpp

+    compare_buf(init_buf, copy_buf, size - extra_off);
+    memset(copy_buf, 0xA5, size * sizeof(*copy_buf));
+
+    extra_off = 11;


Did you chose 11 to not use an aligned value?

Davide wrote this test. I guess that is his intention.

e-ago · 2019-08-07T15:30:31Z

gdrdrv/gdrdrv.c

@@ -339,7 +339,7 @@ static int gdrdrv_release(struct inode *inode, struct file *filp)
    }
    // Check that the caller is the same process that did gdrdrv_open
    if (info->pid != task_pid(current)) {
-        gdr_err("filp is not opened by the current process\n");
+        gdr_dbg("filp is not opened by the current process\n");


This should prevent the dmesg messages I was observing

e-ago · 2019-08-07T15:33:05Z

sanity.cpp

+    ASSERT_EQ(gdr_close(g), 0);
+
+    print_dbg("Trying to read buf_ptr[0] after gdr_close\n");
+    expecting_exception_signal = true;


What about a memory barrier here (and in the next tests at this point) to prevent any reordering?

…anity.cpp

pakmarkthub · 2019-08-07T22:53:41Z

@e-ago I updated the code. Please take a look.

pakmarkthub · 2019-08-09T19:38:13Z

Merged

pakmarkthub requested a review from drossetti July 18, 2019 00:47

pakmarkthub mentioned this pull request Jul 18, 2019

Improve invalidation mechanism and harden security #60

Merged

pakmarkthub changed the title ~~Sanity unittest~~ Sanity test suite Jul 18, 2019

pakmarkthub requested a review from spotluri July 18, 2019 00:50

pakmarkthub force-pushed the dev-sanity-unittest branch from 0a33407 to 1be645a Compare July 29, 2019 22:50

pakmarkthub added 19 commits July 30, 2019 16:22

Created invalidation unit test

06996f3

Changed invalidation_basic to invalidation_two_processes and implemen…

254d50c

…ted invalidation_one_process test case

Created a unit test to ensure invalidation of gdr_map after fork

71d5ae6

Created a unit test to ensure that sharing gdrcopy's fd is not allow

85d58cc

Created unit tests to ensure that mappings of different processes do …

9f5d3fe

…not interfere with each other

Added verbose option and made printing debug message optional in test…

b22729c

…_invalidation

Implemented two more unit tests to ensure that sharing gdr's fd via u…

700120e

…nix socket does not work

Added high-level description to each unit test

98c7a2c

Ensured that parent's and child's mydata are always different in inva…

1377ee5

…lidation_fork_access_after_cumemfree

Added "check" as an dependency in gdrcopy.spec

b659551

Added subunit to the rpm dependency and made test_invalidation compat…

3b86a0a

…ible with libcheck from version 0.9.8

Consolidated unit tests into sanity.cpp

36758fa

Created a unit test to verify that gdr_close properly cleans up the a…

98213dc

…ssociated resources

Used pkg-config to get the correct linker flags, moved the ".cache.mk…

e20b541

…" clean up to gdrdrv, and added *.swp to .gitignore

Removed stale code in sanity.cpp

81ec460

Removed stale comments in sanity.cpp

c8d5bb8

Merged conflicts while rebasing dev-sanity-unittest on invalidate

73a5abb

Fixed the indentation issue in sanity.cpp

7f0c3bb

Added license header, protection from multiple inclusions, and extern…

c801763

… "C" to gdrapi_internal.h

pakmarkthub force-pushed the dev-sanity-unittest branch from 1be645a to c801763 Compare July 30, 2019 23:23

pakmarkthub requested a review from e-ago August 1, 2019 17:42

pakmarkthub added 3 commits August 5, 2019 10:45

Changed all reports of "filp is not opened by the current process" fr…

a1664a4

…om gdr_err to gdr_dbg

Updated README.md

716f1b7

Replaced "validate" with "sanity" in README.md

1746236

pakmarkthub added this to the 2.0 milestone Aug 5, 2019

e-ago reviewed Aug 7, 2019

View reviewed changes

pakmarkthub added 5 commits August 7, 2019 11:06

Added memory fence in sanity.cpp

7b8f74a

Implemented architecture-specific FENCE in sanity.cpp

06de860

Reinitialized d_A for every subtest in data_validation unit test in s…

4ad82e7

…anity.cpp

Added assertion to the return from compare_buf

de48f0f

Fixed race condition inside data_validation

4e0d26a

e-ago approved these changes Aug 8, 2019

View reviewed changes

pakmarkthub closed this Aug 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sanity test suite #71

Sanity test suite #71

pakmarkthub commented Jul 18, 2019

drossetti commented Jul 30, 2019

pakmarkthub commented Jul 30, 2019

e-ago commented Aug 1, 2019 •

edited

Loading

pakmarkthub commented Aug 1, 2019

pakmarkthub commented Aug 6, 2019

e-ago Aug 5, 2019

e-ago Aug 5, 2019

pakmarkthub Aug 7, 2019

e-ago Aug 7, 2019

e-ago Aug 7, 2019

pakmarkthub commented Aug 7, 2019

pakmarkthub commented Aug 9, 2019

Sanity test suite #71

Sanity test suite #71

Conversation

pakmarkthub commented Jul 18, 2019

drossetti commented Jul 30, 2019

pakmarkthub commented Jul 30, 2019

e-ago commented Aug 1, 2019 • edited Loading

pakmarkthub commented Aug 1, 2019

pakmarkthub commented Aug 6, 2019

e-ago Aug 5, 2019

Choose a reason for hiding this comment

e-ago Aug 5, 2019

Choose a reason for hiding this comment

pakmarkthub Aug 7, 2019

Choose a reason for hiding this comment

e-ago Aug 7, 2019

Choose a reason for hiding this comment

e-ago Aug 7, 2019

Choose a reason for hiding this comment

pakmarkthub commented Aug 7, 2019

pakmarkthub commented Aug 9, 2019

e-ago commented Aug 1, 2019 •

edited

Loading