diff mbox series

[v2,3/4] syscalls: accept: Add tst_fd test

Message ID 20231016123320.9865-4-chrubis@suse.cz (mailing list archive)
State New, archived
Headers show
Series Add tst_fd iterator API | expand

Commit Message

Cyril Hrubis Oct. 16, 2023, 12:33 p.m. UTC 
It looks like we return wrong errno on O_PATH file and open_tree() file descriptors.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/accept/.gitignore |  1 +
 testcases/kernel/syscalls/accept/accept01.c |  8 ----
 testcases/kernel/syscalls/accept/accept03.c | 47 +++++++++++++++++++++
 4 files changed, 49 insertions(+), 8 deletions(-)
 create mode 100644 testcases/kernel/syscalls/accept/accept03.c

Comments

Richard Palethorpe Oct. 24, 2023, 9:26 a.m. UTC | #1 
Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> It looks like we return wrong errno on O_PATH file and open_tree() file descriptors.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  runtest/syscalls                            |  1 +
>  testcases/kernel/syscalls/accept/.gitignore |  1 +
>  testcases/kernel/syscalls/accept/accept01.c |  8 ----
>  testcases/kernel/syscalls/accept/accept03.c | 47 +++++++++++++++++++++
>  4 files changed, 49 insertions(+), 8 deletions(-)
>  create mode 100644 testcases/kernel/syscalls/accept/accept03.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 53e519639..55396aad8 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -3,6 +3,7 @@ abort01 abort01
>  
>  accept01 accept01
>  accept02 accept02
> +accept03 accept03
>  
>  accept4_01 accept4_01
>  
> diff --git a/testcases/kernel/syscalls/accept/.gitignore b/testcases/kernel/syscalls/accept/.gitignore
> index 5b1462699..f81d4bec9 100644
> --- a/testcases/kernel/syscalls/accept/.gitignore
> +++ b/testcases/kernel/syscalls/accept/.gitignore
> @@ -1,2 +1,3 @@
>  /accept01
>  /accept02
> +/accept03
> diff --git a/testcases/kernel/syscalls/accept/accept01.c b/testcases/kernel/syscalls/accept/accept01.c
> index 85af0f8af..e5db1dfec 100644
> --- a/testcases/kernel/syscalls/accept/accept01.c
> +++ b/testcases/kernel/syscalls/accept/accept01.c
> @@ -26,7 +26,6 @@
>  struct sockaddr_in sin0, sin1, fsin1;
>  
>  int invalid_socketfd = 400; /* anything that is not an open file */
> -int devnull_fd;
>  int socket_fd;
>  int udp_fd;
>  
> @@ -45,10 +44,6 @@ static struct test_case {
>  		(struct sockaddr *)&fsin1, sizeof(fsin1), EBADF,
>  		"bad file descriptor"
>  	},
> -	{
> -		PF_INET, SOCK_STREAM, 0, &devnull_fd, (struct sockaddr *)&fsin1,
> -		sizeof(fsin1), ENOTSOCK, "fd is not socket"
> -	},
>  	{
>  		PF_INET, SOCK_STREAM, 0, &socket_fd, (struct sockaddr *)3,
>  		sizeof(fsin1), EINVAL, "invalid socket buffer"
> @@ -73,8 +68,6 @@ static void test_setup(void)
>  	sin0.sin_port = 0;
>  	sin0.sin_addr.s_addr = INADDR_ANY;
>  
> -	devnull_fd = SAFE_OPEN("/dev/null", O_WRONLY);
> -
>  	socket_fd = SAFE_SOCKET(PF_INET, SOCK_STREAM, 0);
>  	SAFE_BIND(socket_fd, (struct sockaddr *)&sin0, sizeof(sin0));
>  
> @@ -88,7 +81,6 @@ static void test_setup(void)
>  
>  static void test_cleanup(void)
>  {
> -	SAFE_CLOSE(devnull_fd);
>  	SAFE_CLOSE(socket_fd);
>  	SAFE_CLOSE(udp_fd);
>  }

Is this supposed to be part of the patchset?

I don't mind, but if we are strict, it should be in another commit.

> diff --git a/testcases/kernel/syscalls/accept/accept03.c b/testcases/kernel/syscalls/accept/accept03.c
> new file mode 100644
> index 000000000..084bedaf4
> --- /dev/null
> +++ b/testcases/kernel/syscalls/accept/accept03.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/*
> + * Copyright (C) 2023 Cyril Hrubis <chrubis@suse.cz>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Verify that accept() returns ENOTSOCK for non-socket file descriptors.
> + */
> +
> +#include <sys/socket.h>
> +#include <netinet/in.h>
> +
> +#include "tst_test.h"
> +
> +void check_accept(struct tst_fd *fd)
> +{
> +	struct sockaddr_in addr = {
> +		.sin_family = AF_INET,
> +		.sin_port = 0,
> +		.sin_addr = {.s_addr = INADDR_ANY},
> +	};
> +	socklen_t size = sizeof(addr);
> +
> +	switch (fd->type) {
> +	case TST_FD_UNIX_SOCK:
> +	case TST_FD_INET_SOCK:
> +		return;
> +	default:
> +		break;
> +	}
> +
> +	TST_EXP_FAIL2(accept(fd->fd, (void*)&addr, &size),
> +		ENOTSOCK, "accept() on %s", tst_fd_desc(fd));
> +}
> +
> +static void verify_accept(void)
> +{
> +	TST_FD_FOREACH(fd)
> +		check_accept(&fd);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = verify_accept,
> +};
> -- 
> 2.41.0

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>
Cyril Hrubis Oct. 24, 2023, 9:34 a.m. UTC | #2 
Hi!
> >  int invalid_socketfd = 400; /* anything that is not an open file */
> > -int devnull_fd;
> >  int socket_fd;
> >  int udp_fd;
> >  
> > @@ -45,10 +44,6 @@ static struct test_case {
> >  		(struct sockaddr *)&fsin1, sizeof(fsin1), EBADF,
> >  		"bad file descriptor"
> >  	},
> > -	{
> > -		PF_INET, SOCK_STREAM, 0, &devnull_fd, (struct sockaddr *)&fsin1,
> > -		sizeof(fsin1), ENOTSOCK, "fd is not socket"
> > -	},
> >  	{
> >  		PF_INET, SOCK_STREAM, 0, &socket_fd, (struct sockaddr *)3,
> >  		sizeof(fsin1), EINVAL, "invalid socket buffer"
> > @@ -73,8 +68,6 @@ static void test_setup(void)
> >  	sin0.sin_port = 0;
> >  	sin0.sin_addr.s_addr = INADDR_ANY;
> >  
> > -	devnull_fd = SAFE_OPEN("/dev/null", O_WRONLY);
> > -
> >  	socket_fd = SAFE_SOCKET(PF_INET, SOCK_STREAM, 0);
> >  	SAFE_BIND(socket_fd, (struct sockaddr *)&sin0, sizeof(sin0));
> >  
> > @@ -88,7 +81,6 @@ static void test_setup(void)
> >  
> >  static void test_cleanup(void)
> >  {
> > -	SAFE_CLOSE(devnull_fd);
> >  	SAFE_CLOSE(socket_fd);
> >  	SAFE_CLOSE(udp_fd);
> >  }
> 
> Is this supposed to be part of the patchset?
> 
> I don't mind, but if we are strict, it should be in another commit.

That removes ENOTSOCK test that is now handled in accept03, I suppose I
should have explained that better in the comit message.
diff mbox series

Patch

diff --git a/runtest/syscalls b/runtest/syscalls
index 53e519639..55396aad8 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -3,6 +3,7 @@  abort01 abort01
 
 accept01 accept01
 accept02 accept02
+accept03 accept03
 
 accept4_01 accept4_01
 
diff --git a/testcases/kernel/syscalls/accept/.gitignore b/testcases/kernel/syscalls/accept/.gitignore
index 5b1462699..f81d4bec9 100644
--- a/testcases/kernel/syscalls/accept/.gitignore
+++ b/testcases/kernel/syscalls/accept/.gitignore
@@ -1,2 +1,3 @@ 
 /accept01
 /accept02
+/accept03
diff --git a/testcases/kernel/syscalls/accept/accept01.c b/testcases/kernel/syscalls/accept/accept01.c
index 85af0f8af..e5db1dfec 100644
--- a/testcases/kernel/syscalls/accept/accept01.c
+++ b/testcases/kernel/syscalls/accept/accept01.c
@@ -26,7 +26,6 @@ 
 struct sockaddr_in sin0, sin1, fsin1;
 
 int invalid_socketfd = 400; /* anything that is not an open file */
-int devnull_fd;
 int socket_fd;
 int udp_fd;
 
@@ -45,10 +44,6 @@  static struct test_case {
 		(struct sockaddr *)&fsin1, sizeof(fsin1), EBADF,
 		"bad file descriptor"
 	},
-	{
-		PF_INET, SOCK_STREAM, 0, &devnull_fd, (struct sockaddr *)&fsin1,
-		sizeof(fsin1), ENOTSOCK, "fd is not socket"
-	},
 	{
 		PF_INET, SOCK_STREAM, 0, &socket_fd, (struct sockaddr *)3,
 		sizeof(fsin1), EINVAL, "invalid socket buffer"
@@ -73,8 +68,6 @@  static void test_setup(void)
 	sin0.sin_port = 0;
 	sin0.sin_addr.s_addr = INADDR_ANY;
 
-	devnull_fd = SAFE_OPEN("/dev/null", O_WRONLY);
-
 	socket_fd = SAFE_SOCKET(PF_INET, SOCK_STREAM, 0);
 	SAFE_BIND(socket_fd, (struct sockaddr *)&sin0, sizeof(sin0));
 
@@ -88,7 +81,6 @@  static void test_setup(void)
 
 static void test_cleanup(void)
 {
-	SAFE_CLOSE(devnull_fd);
 	SAFE_CLOSE(socket_fd);
 	SAFE_CLOSE(udp_fd);
 }
diff --git a/testcases/kernel/syscalls/accept/accept03.c b/testcases/kernel/syscalls/accept/accept03.c
new file mode 100644
index 000000000..084bedaf4
--- /dev/null
+++ b/testcases/kernel/syscalls/accept/accept03.c
@@ -0,0 +1,47 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/*
+ * Copyright (C) 2023 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+/*\
+ * [Description]
+ *
+ * Verify that accept() returns ENOTSOCK for non-socket file descriptors.
+ */
+
+#include <sys/socket.h>
+#include <netinet/in.h>
+
+#include "tst_test.h"
+
+void check_accept(struct tst_fd *fd)
+{
+	struct sockaddr_in addr = {
+		.sin_family = AF_INET,
+		.sin_port = 0,
+		.sin_addr = {.s_addr = INADDR_ANY},
+	};
+	socklen_t size = sizeof(addr);
+
+	switch (fd->type) {
+	case TST_FD_UNIX_SOCK:
+	case TST_FD_INET_SOCK:
+		return;
+	default:
+		break;
+	}
+
+	TST_EXP_FAIL2(accept(fd->fd, (void*)&addr, &size),
+		ENOTSOCK, "accept() on %s", tst_fd_desc(fd));
+}
+
+static void verify_accept(void)
+{
+	TST_FD_FOREACH(fd)
+		check_accept(&fd);
+}
+
+static struct tst_test test = {
+	.test_all = verify_accept,
+};