| Message ID | 20200312131531.3615556-3-christian.brauner@ubuntu.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] binderfs: port tests to test harness infrastructure | expand |
On Thu, Mar 12, 2020 at 02:15:31PM +0100, Christian Brauner wrote: > Unprivileged users will be able to create directories in there. The > unprivileged test for /dev wouldn't have worked on most systems. > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Seems good. (Though would a dynamic location be better? mkstemp()-style?) Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > --- > .../filesystems/binderfs/binderfs_test.c | 30 +++++++++---------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c > index 313ffad15614..d6e61998d20a 100644 > --- a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c > +++ b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c > @@ -76,7 +76,7 @@ static void __do_binderfs_test(void) > > change_mountns(); > > - ret = mkdir("/dev/binderfs", 0755); > + ret = mkdir("/tmp/binderfs", 0755); > if (ret < 0) { > if (errno != EEXIST) > ksft_exit_fail_msg( > @@ -86,13 +86,13 @@ static void __do_binderfs_test(void) > keep = true; > } > > - ret = mount(NULL, "/dev/binderfs", "binder", 0, 0); > + ret = mount(NULL, "/tmp/binderfs", "binder", 0, 0); > if (ret < 0) { > if (errno != ENODEV) > ksft_exit_fail_msg("%s - Failed to mount binderfs\n", > strerror(errno)); > > - keep ? : rmdir_protect_errno("/dev/binderfs"); > + keep ? : rmdir_protect_errno("/tmp/binderfs"); > ksft_exit_skip( > "The Android binderfs filesystem is not available\n"); > } > @@ -102,7 +102,7 @@ static void __do_binderfs_test(void) > > memcpy(device.name, "my-binder", strlen("my-binder")); > > - fd = open("/dev/binderfs/binder-control", O_RDONLY | O_CLOEXEC); > + fd = open("/tmp/binderfs/binder-control", O_RDONLY | O_CLOEXEC); > if (fd < 0) > ksft_exit_fail_msg( > "%s - Failed to open binder-control device\n", > @@ -113,7 +113,7 @@ static void __do_binderfs_test(void) > close(fd); > errno = saved_errno; > if (ret < 0) { > - keep ? : rmdir_protect_errno("/dev/binderfs"); > + keep ? : rmdir_protect_errno("/tmp/binderfs"); > ksft_exit_fail_msg( > "%s - Failed to allocate new binder device\n", > strerror(errno)); > @@ -126,9 +126,9 @@ static void __do_binderfs_test(void) > /* binder device allocation test passed */ > ksft_inc_pass_cnt(); > > - fd = open("/dev/binderfs/my-binder", O_CLOEXEC | O_RDONLY); > + fd = open("/tmp/binderfs/my-binder", O_CLOEXEC | O_RDONLY); > if (fd < 0) { > - keep ? : rmdir_protect_errno("/dev/binderfs"); > + keep ? : rmdir_protect_errno("/tmp/binderfs"); > ksft_exit_fail_msg("%s - Failed to open my-binder device\n", > strerror(errno)); > } > @@ -138,7 +138,7 @@ static void __do_binderfs_test(void) > close(fd); > errno = saved_errno; > if (ret < 0) { > - keep ? : rmdir_protect_errno("/dev/binderfs"); > + keep ? : rmdir_protect_errno("/tmp/binderfs"); > ksft_exit_fail_msg( > "%s - Failed to open perform BINDER_VERSION request\n", > strerror(errno)); > @@ -150,9 +150,9 @@ static void __do_binderfs_test(void) > /* binder transaction with binderfs binder device passed */ > ksft_inc_pass_cnt(); > > - ret = unlink("/dev/binderfs/my-binder"); > + ret = unlink("/tmp/binderfs/my-binder"); > if (ret < 0) { > - keep ? : rmdir_protect_errno("/dev/binderfs"); > + keep ? : rmdir_protect_errno("/tmp/binderfs"); > ksft_exit_fail_msg("%s - Failed to delete binder device\n", > strerror(errno)); > } > @@ -160,12 +160,12 @@ static void __do_binderfs_test(void) > /* binder device removal passed */ > ksft_inc_pass_cnt(); > > - ret = unlink("/dev/binderfs/binder-control"); > + ret = unlink("/tmp/binderfs/binder-control"); > if (!ret) { > - keep ? : rmdir_protect_errno("/dev/binderfs"); > + keep ? : rmdir_protect_errno("/tmp/binderfs"); > ksft_exit_fail_msg("Managed to delete binder-control device\n"); > } else if (errno != EPERM) { > - keep ? : rmdir_protect_errno("/dev/binderfs"); > + keep ? : rmdir_protect_errno("/tmp/binderfs"); > ksft_exit_fail_msg( > "%s - Failed to delete binder-control device but exited with unexpected error code\n", > strerror(errno)); > @@ -175,8 +175,8 @@ static void __do_binderfs_test(void) > ksft_inc_xfail_cnt(); > > on_error: > - ret = umount2("/dev/binderfs", MNT_DETACH); > - keep ?: rmdir_protect_errno("/dev/binderfs"); > + ret = umount2("/tmp/binderfs", MNT_DETACH); > + keep ?: rmdir_protect_errno("/tmp/binderfs"); > if (ret < 0) > ksft_exit_fail_msg("%s - Failed to unmount binderfs\n", > strerror(errno)); > -- > 2.25.1 >
On Thu, Mar 12, 2020 at 04:54:25PM -0700, Kees Cook wrote: > On Thu, Mar 12, 2020 at 02:15:31PM +0100, Christian Brauner wrote: > > Unprivileged users will be able to create directories in there. The > > unprivileged test for /dev wouldn't have worked on most systems. > > > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > > Seems good. (Though would a dynamic location be better? > mkstemp()-style?) Yeah, when I originally wrote binderfs that was really more of a test-stub than anything else. I'll see if I can switch to something less hard-coded. :)
diff --git a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c index 313ffad15614..d6e61998d20a 100644 --- a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c +++ b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c @@ -76,7 +76,7 @@ static void __do_binderfs_test(void) change_mountns(); - ret = mkdir("/dev/binderfs", 0755); + ret = mkdir("/tmp/binderfs", 0755); if (ret < 0) { if (errno != EEXIST) ksft_exit_fail_msg( @@ -86,13 +86,13 @@ static void __do_binderfs_test(void) keep = true; } - ret = mount(NULL, "/dev/binderfs", "binder", 0, 0); + ret = mount(NULL, "/tmp/binderfs", "binder", 0, 0); if (ret < 0) { if (errno != ENODEV) ksft_exit_fail_msg("%s - Failed to mount binderfs\n", strerror(errno)); - keep ? : rmdir_protect_errno("/dev/binderfs"); + keep ? : rmdir_protect_errno("/tmp/binderfs"); ksft_exit_skip( "The Android binderfs filesystem is not available\n"); } @@ -102,7 +102,7 @@ static void __do_binderfs_test(void) memcpy(device.name, "my-binder", strlen("my-binder")); - fd = open("/dev/binderfs/binder-control", O_RDONLY | O_CLOEXEC); + fd = open("/tmp/binderfs/binder-control", O_RDONLY | O_CLOEXEC); if (fd < 0) ksft_exit_fail_msg( "%s - Failed to open binder-control device\n", @@ -113,7 +113,7 @@ static void __do_binderfs_test(void) close(fd); errno = saved_errno; if (ret < 0) { - keep ? : rmdir_protect_errno("/dev/binderfs"); + keep ? : rmdir_protect_errno("/tmp/binderfs"); ksft_exit_fail_msg( "%s - Failed to allocate new binder device\n", strerror(errno)); @@ -126,9 +126,9 @@ static void __do_binderfs_test(void) /* binder device allocation test passed */ ksft_inc_pass_cnt(); - fd = open("/dev/binderfs/my-binder", O_CLOEXEC | O_RDONLY); + fd = open("/tmp/binderfs/my-binder", O_CLOEXEC | O_RDONLY); if (fd < 0) { - keep ? : rmdir_protect_errno("/dev/binderfs"); + keep ? : rmdir_protect_errno("/tmp/binderfs"); ksft_exit_fail_msg("%s - Failed to open my-binder device\n", strerror(errno)); } @@ -138,7 +138,7 @@ static void __do_binderfs_test(void) close(fd); errno = saved_errno; if (ret < 0) { - keep ? : rmdir_protect_errno("/dev/binderfs"); + keep ? : rmdir_protect_errno("/tmp/binderfs"); ksft_exit_fail_msg( "%s - Failed to open perform BINDER_VERSION request\n", strerror(errno)); @@ -150,9 +150,9 @@ static void __do_binderfs_test(void) /* binder transaction with binderfs binder device passed */ ksft_inc_pass_cnt(); - ret = unlink("/dev/binderfs/my-binder"); + ret = unlink("/tmp/binderfs/my-binder"); if (ret < 0) { - keep ? : rmdir_protect_errno("/dev/binderfs"); + keep ? : rmdir_protect_errno("/tmp/binderfs"); ksft_exit_fail_msg("%s - Failed to delete binder device\n", strerror(errno)); } @@ -160,12 +160,12 @@ static void __do_binderfs_test(void) /* binder device removal passed */ ksft_inc_pass_cnt(); - ret = unlink("/dev/binderfs/binder-control"); + ret = unlink("/tmp/binderfs/binder-control"); if (!ret) { - keep ? : rmdir_protect_errno("/dev/binderfs"); + keep ? : rmdir_protect_errno("/tmp/binderfs"); ksft_exit_fail_msg("Managed to delete binder-control device\n"); } else if (errno != EPERM) { - keep ? : rmdir_protect_errno("/dev/binderfs"); + keep ? : rmdir_protect_errno("/tmp/binderfs"); ksft_exit_fail_msg( "%s - Failed to delete binder-control device but exited with unexpected error code\n", strerror(errno)); @@ -175,8 +175,8 @@ static void __do_binderfs_test(void) ksft_inc_xfail_cnt(); on_error: - ret = umount2("/dev/binderfs", MNT_DETACH); - keep ?: rmdir_protect_errno("/dev/binderfs"); + ret = umount2("/tmp/binderfs", MNT_DETACH); + keep ?: rmdir_protect_errno("/tmp/binderfs"); if (ret < 0) ksft_exit_fail_msg("%s - Failed to unmount binderfs\n", strerror(errno));
Unprivileged users will be able to create directories in there. The unprivileged test for /dev wouldn't have worked on most systems. Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> --- .../filesystems/binderfs/binderfs_test.c | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-)