linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] selftests: add binderfs selftests
@ 2019-01-16 13:19 Christian Brauner
  2019-01-16 15:54 ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Brauner @ 2019-01-16 13:19 UTC (permalink / raw)
  To: gregkh, tkjos, devel, linux-kernel
  Cc: arve, maco, joel, tkjos, Christian Brauner

This adds the promised selftest for binderfs. It will verify the following
things:
- binderfs mounting works
- binder device allocation works
- performing a binder ioctl() request through a binderfs device works
- binder device removal works
- binder-control removal fails
- binderfs unmounting works

Cc: Todd Kjos <tkjos@google.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 tools/testing/selftests/Makefile              |   1 +
 .../selftests/filesystems/binderfs/.gitignore |   1 +
 .../selftests/filesystems/binderfs/Makefile   |   6 +
 .../filesystems/binderfs/binderfs_test.c      | 120 ++++++++++++++++++
 .../selftests/filesystems/binderfs/config     |   3 +
 5 files changed, 131 insertions(+)
 create mode 100644 tools/testing/selftests/filesystems/binderfs/.gitignore
 create mode 100644 tools/testing/selftests/filesystems/binderfs/Makefile
 create mode 100644 tools/testing/selftests/filesystems/binderfs/binderfs_test.c
 create mode 100644 tools/testing/selftests/filesystems/binderfs/config

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 1a2bd15c5b6e..400ee81a3043 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -10,6 +10,7 @@ TARGETS += drivers/dma-buf
 TARGETS += efivarfs
 TARGETS += exec
 TARGETS += filesystems
+TARGETS += filesystems/binderfs
 TARGETS += firmware
 TARGETS += ftrace
 TARGETS += futex
diff --git a/tools/testing/selftests/filesystems/binderfs/.gitignore b/tools/testing/selftests/filesystems/binderfs/.gitignore
new file mode 100644
index 000000000000..8a5d9bf63dd4
--- /dev/null
+++ b/tools/testing/selftests/filesystems/binderfs/.gitignore
@@ -0,0 +1 @@
+binderfs_test
diff --git a/tools/testing/selftests/filesystems/binderfs/Makefile b/tools/testing/selftests/filesystems/binderfs/Makefile
new file mode 100644
index 000000000000..58cb659b56b4
--- /dev/null
+++ b/tools/testing/selftests/filesystems/binderfs/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+
+CFLAGS += -I../../../../../usr/include/
+TEST_GEN_PROGS := binderfs_test
+
+include ../../lib.mk
diff --git a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
new file mode 100644
index 000000000000..ca4d9b616e84
--- /dev/null
+++ b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <linux/android/binder.h>
+#include <linux/android/binderfs.h>
+#include "../../kselftest.h"
+
+int main(int argc, char *argv[])
+{
+	int fd, ret, saved_errno;
+	size_t len;
+	struct binderfs_device device = { 0 };
+	struct binder_version version = { 0 };
+
+	ret = unshare(CLONE_NEWNS);
+	if (ret < 0)
+		ksft_exit_fail_msg("%s - Failed to unshare mount namespace\n",
+				   strerror(errno));
+
+	ret = mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
+	if (ret < 0)
+		ksft_exit_fail_msg("%s - Failed to mount / as private\n",
+				   strerror(errno));
+
+	ret = mkdir("/dev/binderfs", 0755);
+	if (ret < 0 && errno != EEXIST)
+		ksft_exit_fail_msg(
+			"%s - Failed to create binderfs mountpoint\n",
+			strerror(errno));
+
+	ret = mount(NULL, "/dev/binderfs", "binder", 0, 0);
+	if (ret < 0)
+		ksft_exit_fail_msg("%s - Failed to mount binderfs\n",
+				   strerror(errno));
+
+	/* binderfs mount test passed */
+	ksft_inc_pass_cnt();
+
+	memcpy(device.name, "my-binder", strlen("my-binder"));
+
+	fd = open("/dev/binderfs/binder-control", O_RDONLY | O_CLOEXEC);
+	if (fd < 0)
+		ksft_exit_fail_msg(
+			"%s - Failed to open binder-control device\n",
+			strerror(errno));
+
+	ret = ioctl(fd, BINDER_CTL_ADD, &device);
+	saved_errno = errno;
+	close(fd);
+	errno = saved_errno;
+	if (ret < 0)
+		ksft_exit_fail_msg(
+			"%s - Failed to allocate new binder device\n",
+			strerror(errno));
+
+	printf("Allocated new binder device with major %d, minor %d, and name %s\n",
+	       device.major, device.minor, device.name);
+
+	/* binder device allocation test passed */
+	ksft_inc_pass_cnt();
+
+	fd = open("/dev/binderfs/my-binder", O_CLOEXEC | O_RDONLY);
+	if (fd < 0)
+		ksft_exit_fail_msg("%s - Failed to open my-binder device\n",
+				   strerror(errno));
+
+	ret = ioctl(fd, BINDER_VERSION, &version);
+	saved_errno = errno;
+	close(fd);
+	errno = saved_errno;
+	if (ret < 0)
+		ksft_exit_fail_msg(
+			"%s - Failed to open perform BINDER_VERSION request\n",
+			strerror(errno));
+
+	printf("Detected binder version: %d\n", version.protocol_version);
+
+	/* binder transaction with binderfs binder device passed */
+	ksft_inc_pass_cnt();
+
+	ret = unlink("/dev/binderfs/my-binder");
+	if (ret < 0)
+		ksft_exit_fail_msg("%s - Failed to delete binder device\n",
+				   strerror(errno));
+
+	/* binder device removal passed */
+	ksft_inc_pass_cnt();
+
+	ret = unlink("/dev/binderfs/binder-control");
+	if (!ret)
+		ksft_exit_fail_msg("Managed to delete binder-control device\n");
+	else if (errno != EPERM)
+		ksft_exit_fail_msg(
+			"%s - Failed to delete binder-control device but exited with unexpected error code\n",
+			strerror(errno));
+
+	/* binder-control device removal failed as expected */
+	ksft_inc_xfail_cnt();
+
+	ret = umount2("/dev/binderfs", MNT_DETACH);
+	if (ret < 0)
+		ksft_exit_fail_msg("%s - Failed to unmount binderfs\n",
+				   strerror(errno));
+
+	/* binderfs unmount test passed */
+	ksft_inc_pass_cnt();
+
+	ksft_exit_pass();
+}
diff --git a/tools/testing/selftests/filesystems/binderfs/config b/tools/testing/selftests/filesystems/binderfs/config
new file mode 100644
index 000000000000..02dd6cc9cf99
--- /dev/null
+++ b/tools/testing/selftests/filesystems/binderfs/config
@@ -0,0 +1,3 @@
+CONFIG_ANDROID=y
+CONFIG_ANDROID_BINDERFS=y
+CONFIG_ANDROID_BINDER_IPC=y
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests: add binderfs selftests
  2019-01-16 13:19 [PATCH] selftests: add binderfs selftests Christian Brauner
@ 2019-01-16 15:54 ` Greg KH
  2019-01-16 16:21   ` Christian Brauner
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2019-01-16 15:54 UTC (permalink / raw)
  To: Christian Brauner; +Cc: tkjos, devel, linux-kernel, joel, arve, maco, tkjos

On Wed, Jan 16, 2019 at 02:19:03PM +0100, Christian Brauner wrote:
> This adds the promised selftest for binderfs. It will verify the following
> things:
> - binderfs mounting works
> - binder device allocation works
> - performing a binder ioctl() request through a binderfs device works
> - binder device removal works
> - binder-control removal fails
> - binderfs unmounting works
> 
> Cc: Todd Kjos <tkjos@google.com>
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
>  tools/testing/selftests/Makefile              |   1 +
>  .../selftests/filesystems/binderfs/.gitignore |   1 +
>  .../selftests/filesystems/binderfs/Makefile   |   6 +
>  .../filesystems/binderfs/binderfs_test.c      | 120 ++++++++++++++++++
>  .../selftests/filesystems/binderfs/config     |   3 +
>  5 files changed, 131 insertions(+)
>  create mode 100644 tools/testing/selftests/filesystems/binderfs/.gitignore
>  create mode 100644 tools/testing/selftests/filesystems/binderfs/Makefile
>  create mode 100644 tools/testing/selftests/filesystems/binderfs/binderfs_test.c
>  create mode 100644 tools/testing/selftests/filesystems/binderfs/config
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 1a2bd15c5b6e..400ee81a3043 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -10,6 +10,7 @@ TARGETS += drivers/dma-buf
>  TARGETS += efivarfs
>  TARGETS += exec
>  TARGETS += filesystems
> +TARGETS += filesystems/binderfs
>  TARGETS += firmware
>  TARGETS += ftrace
>  TARGETS += futex
> diff --git a/tools/testing/selftests/filesystems/binderfs/.gitignore b/tools/testing/selftests/filesystems/binderfs/.gitignore
> new file mode 100644
> index 000000000000..8a5d9bf63dd4
> --- /dev/null
> +++ b/tools/testing/selftests/filesystems/binderfs/.gitignore
> @@ -0,0 +1 @@
> +binderfs_test
> diff --git a/tools/testing/selftests/filesystems/binderfs/Makefile b/tools/testing/selftests/filesystems/binderfs/Makefile
> new file mode 100644
> index 000000000000..58cb659b56b4
> --- /dev/null
> +++ b/tools/testing/selftests/filesystems/binderfs/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +CFLAGS += -I../../../../../usr/include/
> +TEST_GEN_PROGS := binderfs_test
> +
> +include ../../lib.mk
> diff --git a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
> new file mode 100644
> index 000000000000..ca4d9b616e84
> --- /dev/null
> +++ b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define _GNU_SOURCE
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <sched.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +#include <sys/mount.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <linux/android/binder.h>
> +#include <linux/android/binderfs.h>
> +#include "../../kselftest.h"
> +
> +int main(int argc, char *argv[])
> +{
> +	int fd, ret, saved_errno;
> +	size_t len;
> +	struct binderfs_device device = { 0 };
> +	struct binder_version version = { 0 };
> +
> +	ret = unshare(CLONE_NEWNS);
> +	if (ret < 0)
> +		ksft_exit_fail_msg("%s - Failed to unshare mount namespace\n",
> +				   strerror(errno));
> +
> +	ret = mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> +	if (ret < 0)
> +		ksft_exit_fail_msg("%s - Failed to mount / as private\n",
> +				   strerror(errno));
> +
> +	ret = mkdir("/dev/binderfs", 0755);
> +	if (ret < 0 && errno != EEXIST)
> +		ksft_exit_fail_msg(
> +			"%s - Failed to create binderfs mountpoint\n",
> +			strerror(errno));
> +
> +	ret = mount(NULL, "/dev/binderfs", "binder", 0, 0);
> +	if (ret < 0)
> +		ksft_exit_fail_msg("%s - Failed to mount binderfs\n",
> +				   strerror(errno));

Can you check first to see if the kernel under test really even has
binderfs in it?  If not, you need to just abort the test, not fail it,
so as to allow newer versions of kselftests to run on older kernels.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests: add binderfs selftests
  2019-01-16 15:54 ` Greg KH
@ 2019-01-16 16:21   ` Christian Brauner
  2019-01-16 16:42     ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Brauner @ 2019-01-16 16:21 UTC (permalink / raw)
  To: Greg KH; +Cc: tkjos, devel, linux-kernel, joel, arve, maco, tkjos

On January 16, 2019 5:54:50 PM GMT+02:00, Greg KH <gregkh@linuxfoundation.org> wrote:
>On Wed, Jan 16, 2019 at 02:19:03PM +0100, Christian Brauner wrote:
>> This adds the promised selftest for binderfs. It will verify the
>following
>> things:
>> - binderfs mounting works
>> - binder device allocation works
>> - performing a binder ioctl() request through a binderfs device works
>> - binder device removal works
>> - binder-control removal fails
>> - binderfs unmounting works
>> 
>> Cc: Todd Kjos <tkjos@google.com>
>> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
>> ---
>>  tools/testing/selftests/Makefile              |   1 +
>>  .../selftests/filesystems/binderfs/.gitignore |   1 +
>>  .../selftests/filesystems/binderfs/Makefile   |   6 +
>>  .../filesystems/binderfs/binderfs_test.c      | 120
>++++++++++++++++++
>>  .../selftests/filesystems/binderfs/config     |   3 +
>>  5 files changed, 131 insertions(+)
>>  create mode 100644
>tools/testing/selftests/filesystems/binderfs/.gitignore
>>  create mode 100644
>tools/testing/selftests/filesystems/binderfs/Makefile
>>  create mode 100644
>tools/testing/selftests/filesystems/binderfs/binderfs_test.c
>>  create mode 100644
>tools/testing/selftests/filesystems/binderfs/config
>> 
>> diff --git a/tools/testing/selftests/Makefile
>b/tools/testing/selftests/Makefile
>> index 1a2bd15c5b6e..400ee81a3043 100644
>> --- a/tools/testing/selftests/Makefile
>> +++ b/tools/testing/selftests/Makefile
>> @@ -10,6 +10,7 @@ TARGETS += drivers/dma-buf
>>  TARGETS += efivarfs
>>  TARGETS += exec
>>  TARGETS += filesystems
>> +TARGETS += filesystems/binderfs
>>  TARGETS += firmware
>>  TARGETS += ftrace
>>  TARGETS += futex
>> diff --git a/tools/testing/selftests/filesystems/binderfs/.gitignore
>b/tools/testing/selftests/filesystems/binderfs/.gitignore
>> new file mode 100644
>> index 000000000000..8a5d9bf63dd4
>> --- /dev/null
>> +++ b/tools/testing/selftests/filesystems/binderfs/.gitignore
>> @@ -0,0 +1 @@
>> +binderfs_test
>> diff --git a/tools/testing/selftests/filesystems/binderfs/Makefile
>b/tools/testing/selftests/filesystems/binderfs/Makefile
>> new file mode 100644
>> index 000000000000..58cb659b56b4
>> --- /dev/null
>> +++ b/tools/testing/selftests/filesystems/binderfs/Makefile
>> @@ -0,0 +1,6 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +CFLAGS += -I../../../../../usr/include/
>> +TEST_GEN_PROGS := binderfs_test
>> +
>> +include ../../lib.mk
>> diff --git
>a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
>b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
>> new file mode 100644
>> index 000000000000..ca4d9b616e84
>> --- /dev/null
>> +++ b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
>> @@ -0,0 +1,120 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#define _GNU_SOURCE
>> +#include <errno.h>
>> +#include <fcntl.h>
>> +#include <sched.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <sys/ioctl.h>
>> +#include <sys/mount.h>
>> +#include <sys/stat.h>
>> +#include <sys/types.h>
>> +#include <unistd.h>
>> +#include <linux/android/binder.h>
>> +#include <linux/android/binderfs.h>
>> +#include "../../kselftest.h"
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +	int fd, ret, saved_errno;
>> +	size_t len;
>> +	struct binderfs_device device = { 0 };
>> +	struct binder_version version = { 0 };
>> +
>> +	ret = unshare(CLONE_NEWNS);
>> +	if (ret < 0)
>> +		ksft_exit_fail_msg("%s - Failed to unshare mount namespace\n",
>> +				   strerror(errno));
>> +
>> +	ret = mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
>> +	if (ret < 0)
>> +		ksft_exit_fail_msg("%s - Failed to mount / as private\n",
>> +				   strerror(errno));
>> +
>> +	ret = mkdir("/dev/binderfs", 0755);
>> +	if (ret < 0 && errno != EEXIST)
>> +		ksft_exit_fail_msg(
>> +			"%s - Failed to create binderfs mountpoint\n",
>> +			strerror(errno));
>> +
>> +	ret = mount(NULL, "/dev/binderfs", "binder", 0, 0);
>> +	if (ret < 0)
>> +		ksft_exit_fail_msg("%s - Failed to mount binderfs\n",
>> +				   strerror(errno));
>
>Can you check first to see if the kernel under test really even has
>binderfs in it?  If not, you need to just

Hm, I thought that's what the "config" file was for?
E.g. a conditional compile or is this just a hint to the
user what's needed to run the test?

Thanks!
Christian

Christian

 abort the test, not fail it,
>so as to allow newer versions of kselftests to run on older kernels.
>
>thanks,
>
>greg k-h


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests: add binderfs selftests
  2019-01-16 16:21   ` Christian Brauner
@ 2019-01-16 16:42     ` Greg KH
  2019-01-16 16:57       ` Christian Brauner
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2019-01-16 16:42 UTC (permalink / raw)
  To: Christian Brauner; +Cc: tkjos, devel, linux-kernel, joel, arve, maco, tkjos

On Wed, Jan 16, 2019 at 06:21:12PM +0200, Christian Brauner wrote:
> On January 16, 2019 5:54:50 PM GMT+02:00, Greg KH <gregkh@linuxfoundation.org> wrote:
> >On Wed, Jan 16, 2019 at 02:19:03PM +0100, Christian Brauner wrote:
> >> This adds the promised selftest for binderfs. It will verify the
> >following
> >> things:
> >> - binderfs mounting works
> >> - binder device allocation works
> >> - performing a binder ioctl() request through a binderfs device works
> >> - binder device removal works
> >> - binder-control removal fails
> >> - binderfs unmounting works
> >> 
> >> Cc: Todd Kjos <tkjos@google.com>
> >> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> >> ---
> >>  tools/testing/selftests/Makefile              |   1 +
> >>  .../selftests/filesystems/binderfs/.gitignore |   1 +
> >>  .../selftests/filesystems/binderfs/Makefile   |   6 +
> >>  .../filesystems/binderfs/binderfs_test.c      | 120
> >++++++++++++++++++
> >>  .../selftests/filesystems/binderfs/config     |   3 +
> >>  5 files changed, 131 insertions(+)
> >>  create mode 100644
> >tools/testing/selftests/filesystems/binderfs/.gitignore
> >>  create mode 100644
> >tools/testing/selftests/filesystems/binderfs/Makefile
> >>  create mode 100644
> >tools/testing/selftests/filesystems/binderfs/binderfs_test.c
> >>  create mode 100644
> >tools/testing/selftests/filesystems/binderfs/config
> >> 
> >> diff --git a/tools/testing/selftests/Makefile
> >b/tools/testing/selftests/Makefile
> >> index 1a2bd15c5b6e..400ee81a3043 100644
> >> --- a/tools/testing/selftests/Makefile
> >> +++ b/tools/testing/selftests/Makefile
> >> @@ -10,6 +10,7 @@ TARGETS += drivers/dma-buf
> >>  TARGETS += efivarfs
> >>  TARGETS += exec
> >>  TARGETS += filesystems
> >> +TARGETS += filesystems/binderfs
> >>  TARGETS += firmware
> >>  TARGETS += ftrace
> >>  TARGETS += futex
> >> diff --git a/tools/testing/selftests/filesystems/binderfs/.gitignore
> >b/tools/testing/selftests/filesystems/binderfs/.gitignore
> >> new file mode 100644
> >> index 000000000000..8a5d9bf63dd4
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/filesystems/binderfs/.gitignore
> >> @@ -0,0 +1 @@
> >> +binderfs_test
> >> diff --git a/tools/testing/selftests/filesystems/binderfs/Makefile
> >b/tools/testing/selftests/filesystems/binderfs/Makefile
> >> new file mode 100644
> >> index 000000000000..58cb659b56b4
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/filesystems/binderfs/Makefile
> >> @@ -0,0 +1,6 @@
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +
> >> +CFLAGS += -I../../../../../usr/include/
> >> +TEST_GEN_PROGS := binderfs_test
> >> +
> >> +include ../../lib.mk
> >> diff --git
> >a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
> >b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
> >> new file mode 100644
> >> index 000000000000..ca4d9b616e84
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
> >> @@ -0,0 +1,120 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +
> >> +#define _GNU_SOURCE
> >> +#include <errno.h>
> >> +#include <fcntl.h>
> >> +#include <sched.h>
> >> +#include <stdio.h>
> >> +#include <stdlib.h>
> >> +#include <string.h>
> >> +#include <sys/ioctl.h>
> >> +#include <sys/mount.h>
> >> +#include <sys/stat.h>
> >> +#include <sys/types.h>
> >> +#include <unistd.h>
> >> +#include <linux/android/binder.h>
> >> +#include <linux/android/binderfs.h>
> >> +#include "../../kselftest.h"
> >> +
> >> +int main(int argc, char *argv[])
> >> +{
> >> +	int fd, ret, saved_errno;
> >> +	size_t len;
> >> +	struct binderfs_device device = { 0 };
> >> +	struct binder_version version = { 0 };
> >> +
> >> +	ret = unshare(CLONE_NEWNS);
> >> +	if (ret < 0)
> >> +		ksft_exit_fail_msg("%s - Failed to unshare mount namespace\n",
> >> +				   strerror(errno));
> >> +
> >> +	ret = mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> >> +	if (ret < 0)
> >> +		ksft_exit_fail_msg("%s - Failed to mount / as private\n",
> >> +				   strerror(errno));
> >> +
> >> +	ret = mkdir("/dev/binderfs", 0755);
> >> +	if (ret < 0 && errno != EEXIST)
> >> +		ksft_exit_fail_msg(
> >> +			"%s - Failed to create binderfs mountpoint\n",
> >> +			strerror(errno));
> >> +
> >> +	ret = mount(NULL, "/dev/binderfs", "binder", 0, 0);
> >> +	if (ret < 0)
> >> +		ksft_exit_fail_msg("%s - Failed to mount binderfs\n",
> >> +				   strerror(errno));
> >
> >Can you check first to see if the kernel under test really even has
> >binderfs in it?  If not, you need to just
> 
> Hm, I thought that's what the "config" file was for?
> E.g. a conditional compile or is this just a hint to the
> user what's needed to run the test?

I do not know.  What happens if you try to run this test on a kernel
without that config option?  Does it not work as the framework correctly
detects this?

And new kernel self tests should cc: the ksefltest mailing list and the
maintainer of that subsystem so that they can review the test for things
like this.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests: add binderfs selftests
  2019-01-16 16:42     ` Greg KH
@ 2019-01-16 16:57       ` Christian Brauner
  2019-01-16 22:12         ` Christian Brauner
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Brauner @ 2019-01-16 16:57 UTC (permalink / raw)
  To: Greg KH; +Cc: tkjos, devel, linux-kernel, joel, arve, maco, tkjos

On January 16, 2019 6:42:20 PM GMT+02:00, Greg KH <gregkh@linuxfoundation.org> wrote:
>On Wed, Jan 16, 2019 at 06:21:12PM +0200, Christian Brauner wrote:
>> On January 16, 2019 5:54:50 PM GMT+02:00, Greg KH
><gregkh@linuxfoundation.org> wrote:
>> >On Wed, Jan 16, 2019 at 02:19:03PM +0100, Christian Brauner wrote:
>> >> This adds the promised selftest for binderfs. It will verify the
>> >following
>> >> things:
>> >> - binderfs mounting works
>> >> - binder device allocation works
>> >> - performing a binder ioctl() request through a binderfs device
>works
>> >> - binder device removal works
>> >> - binder-control removal fails
>> >> - binderfs unmounting works
>> >> 
>> >> Cc: Todd Kjos <tkjos@google.com>
>> >> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
>> >> ---
>> >>  tools/testing/selftests/Makefile              |   1 +
>> >>  .../selftests/filesystems/binderfs/.gitignore |   1 +
>> >>  .../selftests/filesystems/binderfs/Makefile   |   6 +
>> >>  .../filesystems/binderfs/binderfs_test.c      | 120
>> >++++++++++++++++++
>> >>  .../selftests/filesystems/binderfs/config     |   3 +
>> >>  5 files changed, 131 insertions(+)
>> >>  create mode 100644
>> >tools/testing/selftests/filesystems/binderfs/.gitignore
>> >>  create mode 100644
>> >tools/testing/selftests/filesystems/binderfs/Makefile
>> >>  create mode 100644
>> >tools/testing/selftests/filesystems/binderfs/binderfs_test.c
>> >>  create mode 100644
>> >tools/testing/selftests/filesystems/binderfs/config
>> >> 
>> >> diff --git a/tools/testing/selftests/Makefile
>> >b/tools/testing/selftests/Makefile
>> >> index 1a2bd15c5b6e..400ee81a3043 100644
>> >> --- a/tools/testing/selftests/Makefile
>> >> +++ b/tools/testing/selftests/Makefile
>> >> @@ -10,6 +10,7 @@ TARGETS += drivers/dma-buf
>> >>  TARGETS += efivarfs
>> >>  TARGETS += exec
>> >>  TARGETS += filesystems
>> >> +TARGETS += filesystems/binderfs
>> >>  TARGETS += firmware
>> >>  TARGETS += ftrace
>> >>  TARGETS += futex
>> >> diff --git
>a/tools/testing/selftests/filesystems/binderfs/.gitignore
>> >b/tools/testing/selftests/filesystems/binderfs/.gitignore
>> >> new file mode 100644
>> >> index 000000000000..8a5d9bf63dd4
>> >> --- /dev/null
>> >> +++ b/tools/testing/selftests/filesystems/binderfs/.gitignore
>> >> @@ -0,0 +1 @@
>> >> +binderfs_test
>> >> diff --git a/tools/testing/selftests/filesystems/binderfs/Makefile
>> >b/tools/testing/selftests/filesystems/binderfs/Makefile
>> >> new file mode 100644
>> >> index 000000000000..58cb659b56b4
>> >> --- /dev/null
>> >> +++ b/tools/testing/selftests/filesystems/binderfs/Makefile
>> >> @@ -0,0 +1,6 @@
>> >> +# SPDX-License-Identifier: GPL-2.0
>> >> +
>> >> +CFLAGS += -I../../../../../usr/include/
>> >> +TEST_GEN_PROGS := binderfs_test
>> >> +
>> >> +include ../../lib.mk
>> >> diff --git
>> >a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
>> >b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
>> >> new file mode 100644
>> >> index 000000000000..ca4d9b616e84
>> >> --- /dev/null
>> >> +++ b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
>> >> @@ -0,0 +1,120 @@
>> >> +// SPDX-License-Identifier: GPL-2.0
>> >> +
>> >> +#define _GNU_SOURCE
>> >> +#include <errno.h>
>> >> +#include <fcntl.h>
>> >> +#include <sched.h>
>> >> +#include <stdio.h>
>> >> +#include <stdlib.h>
>> >> +#include <string.h>
>> >> +#include <sys/ioctl.h>
>> >> +#include <sys/mount.h>
>> >> +#include <sys/stat.h>
>> >> +#include <sys/types.h>
>> >> +#include <unistd.h>
>> >> +#include <linux/android/binder.h>
>> >> +#include <linux/android/binderfs.h>
>> >> +#include "../../kselftest.h"
>> >> +
>> >> +int main(int argc, char *argv[])
>> >> +{
>> >> +	int fd, ret, saved_errno;
>> >> +	size_t len;
>> >> +	struct binderfs_device device = { 0 };
>> >> +	struct binder_version version = { 0 };
>> >> +
>> >> +	ret = unshare(CLONE_NEWNS);
>> >> +	if (ret < 0)
>> >> +		ksft_exit_fail_msg("%s - Failed to unshare mount namespace\n",
>> >> +				   strerror(errno));
>> >> +
>> >> +	ret = mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
>> >> +	if (ret < 0)
>> >> +		ksft_exit_fail_msg("%s - Failed to mount / as private\n",
>> >> +				   strerror(errno));
>> >> +
>> >> +	ret = mkdir("/dev/binderfs", 0755);
>> >> +	if (ret < 0 && errno != EEXIST)
>> >> +		ksft_exit_fail_msg(
>> >> +			"%s - Failed to create binderfs mountpoint\n",
>> >> +			strerror(errno));
>> >> +
>> >> +	ret = mount(NULL, "/dev/binderfs", "binder", 0, 0);
>> >> +	if (ret < 0)
>> >> +		ksft_exit_fail_msg("%s - Failed to mount binderfs\n",
>> >> +				   strerror(errno));
>> >
>> >Can you check first to see if the kernel under test really even has
>> >binderfs in it?  If not, you need to just
>> 
>> Hm, I thought that's what the "config" file was for?
>> E.g. a conditional compile or is this just a hint to the
>> user what's needed to run the test?
>
>I do not know.  What happens if you try to run this test on a kernel
>without that config option?  Does it not work as the framework
>correctly
>detects this?

I did test and it worked fine but let me check by setting up a clean vm and retry.

>
>And new kernel self tests should cc: the ksefltest mailing list and the
>maintainer of that subsystem so that they can review the test for
>things
>like this.

Thanks, missed them in my Cc list.

Christian


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests: add binderfs selftests
  2019-01-16 16:57       ` Christian Brauner
@ 2019-01-16 22:12         ` Christian Brauner
  0 siblings, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2019-01-16 22:12 UTC (permalink / raw)
  To: Greg KH; +Cc: tkjos, devel, linux-kernel, joel, arve, maco, tkjos

On Wed, Jan 16, 2019 at 06:57:10PM +0200, Christian Brauner wrote:
> On January 16, 2019 6:42:20 PM GMT+02:00, Greg KH <gregkh@linuxfoundation.org> wrote:
> >On Wed, Jan 16, 2019 at 06:21:12PM +0200, Christian Brauner wrote:
> >> On January 16, 2019 5:54:50 PM GMT+02:00, Greg KH
> ><gregkh@linuxfoundation.org> wrote:
> >> >On Wed, Jan 16, 2019 at 02:19:03PM +0100, Christian Brauner wrote:
> >> >> This adds the promised selftest for binderfs. It will verify the
> >> >following
> >> >> things:
> >> >> - binderfs mounting works
> >> >> - binder device allocation works
> >> >> - performing a binder ioctl() request through a binderfs device
> >works
> >> >> - binder device removal works
> >> >> - binder-control removal fails
> >> >> - binderfs unmounting works
> >> >> 
> >> >> Cc: Todd Kjos <tkjos@google.com>
> >> >> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> >> >> ---
> >> >>  tools/testing/selftests/Makefile              |   1 +
> >> >>  .../selftests/filesystems/binderfs/.gitignore |   1 +
> >> >>  .../selftests/filesystems/binderfs/Makefile   |   6 +
> >> >>  .../filesystems/binderfs/binderfs_test.c      | 120
> >> >++++++++++++++++++
> >> >>  .../selftests/filesystems/binderfs/config     |   3 +
> >> >>  5 files changed, 131 insertions(+)
> >> >>  create mode 100644
> >> >tools/testing/selftests/filesystems/binderfs/.gitignore
> >> >>  create mode 100644
> >> >tools/testing/selftests/filesystems/binderfs/Makefile
> >> >>  create mode 100644
> >> >tools/testing/selftests/filesystems/binderfs/binderfs_test.c
> >> >>  create mode 100644
> >> >tools/testing/selftests/filesystems/binderfs/config
> >> >> 
> >> >> diff --git a/tools/testing/selftests/Makefile
> >> >b/tools/testing/selftests/Makefile
> >> >> index 1a2bd15c5b6e..400ee81a3043 100644
> >> >> --- a/tools/testing/selftests/Makefile
> >> >> +++ b/tools/testing/selftests/Makefile
> >> >> @@ -10,6 +10,7 @@ TARGETS += drivers/dma-buf
> >> >>  TARGETS += efivarfs
> >> >>  TARGETS += exec
> >> >>  TARGETS += filesystems
> >> >> +TARGETS += filesystems/binderfs
> >> >>  TARGETS += firmware
> >> >>  TARGETS += ftrace
> >> >>  TARGETS += futex
> >> >> diff --git
> >a/tools/testing/selftests/filesystems/binderfs/.gitignore
> >> >b/tools/testing/selftests/filesystems/binderfs/.gitignore
> >> >> new file mode 100644
> >> >> index 000000000000..8a5d9bf63dd4
> >> >> --- /dev/null
> >> >> +++ b/tools/testing/selftests/filesystems/binderfs/.gitignore
> >> >> @@ -0,0 +1 @@
> >> >> +binderfs_test
> >> >> diff --git a/tools/testing/selftests/filesystems/binderfs/Makefile
> >> >b/tools/testing/selftests/filesystems/binderfs/Makefile
> >> >> new file mode 100644
> >> >> index 000000000000..58cb659b56b4
> >> >> --- /dev/null
> >> >> +++ b/tools/testing/selftests/filesystems/binderfs/Makefile
> >> >> @@ -0,0 +1,6 @@
> >> >> +# SPDX-License-Identifier: GPL-2.0
> >> >> +
> >> >> +CFLAGS += -I../../../../../usr/include/
> >> >> +TEST_GEN_PROGS := binderfs_test
> >> >> +
> >> >> +include ../../lib.mk
> >> >> diff --git
> >> >a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
> >> >b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
> >> >> new file mode 100644
> >> >> index 000000000000..ca4d9b616e84
> >> >> --- /dev/null
> >> >> +++ b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
> >> >> @@ -0,0 +1,120 @@
> >> >> +// SPDX-License-Identifier: GPL-2.0
> >> >> +
> >> >> +#define _GNU_SOURCE
> >> >> +#include <errno.h>
> >> >> +#include <fcntl.h>
> >> >> +#include <sched.h>
> >> >> +#include <stdio.h>
> >> >> +#include <stdlib.h>
> >> >> +#include <string.h>
> >> >> +#include <sys/ioctl.h>
> >> >> +#include <sys/mount.h>
> >> >> +#include <sys/stat.h>
> >> >> +#include <sys/types.h>
> >> >> +#include <unistd.h>
> >> >> +#include <linux/android/binder.h>
> >> >> +#include <linux/android/binderfs.h>
> >> >> +#include "../../kselftest.h"
> >> >> +
> >> >> +int main(int argc, char *argv[])
> >> >> +{
> >> >> +	int fd, ret, saved_errno;
> >> >> +	size_t len;
> >> >> +	struct binderfs_device device = { 0 };
> >> >> +	struct binder_version version = { 0 };
> >> >> +
> >> >> +	ret = unshare(CLONE_NEWNS);
> >> >> +	if (ret < 0)
> >> >> +		ksft_exit_fail_msg("%s - Failed to unshare mount namespace\n",
> >> >> +				   strerror(errno));
> >> >> +
> >> >> +	ret = mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> >> >> +	if (ret < 0)
> >> >> +		ksft_exit_fail_msg("%s - Failed to mount / as private\n",
> >> >> +				   strerror(errno));
> >> >> +
> >> >> +	ret = mkdir("/dev/binderfs", 0755);
> >> >> +	if (ret < 0 && errno != EEXIST)
> >> >> +		ksft_exit_fail_msg(
> >> >> +			"%s - Failed to create binderfs mountpoint\n",
> >> >> +			strerror(errno));
> >> >> +
> >> >> +	ret = mount(NULL, "/dev/binderfs", "binder", 0, 0);
> >> >> +	if (ret < 0)
> >> >> +		ksft_exit_fail_msg("%s - Failed to mount binderfs\n",
> >> >> +				   strerror(errno));
> >> >
> >> >Can you check first to see if the kernel under test really even has
> >> >binderfs in it?  If not, you need to just
> >> 
> >> Hm, I thought that's what the "config" file was for?
> >> E.g. a conditional compile or is this just a hint to the
> >> user what's needed to run the test?
> >
> >I do not know.  What happens if you try to run this test on a kernel
> >without that config option?  Does it not work as the framework
> >correctly
> >detects this?
> 
> I did test and it worked fine but let me check by setting up a clean vm and retry.

The compile always happens and always succeeds but the test will fail if
run on a kernel that does not have binderfs. So the mount() call should
check for ret == ENODEV and then exit with skip.

Thanks!
Christian

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-01-16 22:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-16 13:19 [PATCH] selftests: add binderfs selftests Christian Brauner
2019-01-16 15:54 ` Greg KH
2019-01-16 16:21   ` Christian Brauner
2019-01-16 16:42     ` Greg KH
2019-01-16 16:57       ` Christian Brauner
2019-01-16 22:12         ` Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).