On Thu, Nov 11, 2021 at 12:09:41PM +0300, Vitaliy Makkoveev wrote:
> Can I propose to commit this diff before? It reorders soclose() to
> destroy PCB before `so_q0' and `so_q' cleanup.
OK bluhm@
> Also the tool to test the diff:
What happens when you run the tool with the current code? Does the
kernel crash? Are there some inconsistencies that userland could
detect?
If yes, we could convert it to a regress test.
bluhm
> #include <sys/types.h>
> #include <sys/socket.h>
> #include <sys/un.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <err.h>
> #include <pthread.h>
> #include <string.h>
> #include <unistd.h>
>
> #include <arpa/inet.h>
> #include <netinet/in.h>
>
> #define NTHR (10)
> #define NCONNECT (10)
> #define NACCEPT ((NTHR * NCONNECT) / 2)
>
> static void *
> thr_conn(void *arg)
> {
> struct sockaddr_storage *ss = arg;
> int s[NCONNECT];
> size_t hd = 0, tl = 0;
>
> while (1) {
> if ((s[hd] = socket(ss->ss_family, SOCK_STREAM, 0)) < 0)
> continue;
>
> if (connect(s[hd], (struct sockaddr *)ss, ss->ss_len) < 0) {
> close(s[hd]);
> continue;
> }
>
> if ((hd = (hd + 1) % NCONNECT) == tl) {
> close(s[tl]);
> tl = (tl + 1) % NCONNECT;
> }
> }
>
> return NULL;
> }
>
> static void
> usage(void)
> {
> extern char *__progname;
>
> fprintf(stderr, "Usage %s: unix | inet | inet6\n", __progname);
> exit(1);
> }
>
> int
> main(int argc, char *argv[])
> {
> struct sockaddr_storage ss;
> size_t i;
> int s;
>
> if (argc != 2) {
> usage();
> }
>
> memset(&ss, 0, sizeof(ss));
>
> if (strcmp(argv[1], "unix") == 0) {
> struct sockaddr_un *sun;
>
> sun = (struct sockaddr_un *)&ss;
> sun->sun_len = sizeof(*sun);
> sun->sun_family = AF_UNIX;
> snprintf(sun->sun_path, sizeof(sun->sun_path) - 1,
> "/tmp/socket%d", getpid());
> } else if (strcmp(argv[1], "inet") == 0) {
> struct sockaddr_in *sin;
>
> sin = (struct sockaddr_in *)&ss;
> sin->sin_len = sizeof(*sin);
> sin->sin_family = AF_INET;
> sin->sin_port = htons(10000+(getpid()%10000));
> if (inet_pton(AF_INET, "127.0.0.1", &sin->sin_addr) <= 0)
> errx(1, "inet_pton()");
> } else if (strcmp(argv[1], "inet6") == 0) {
> struct sockaddr_in6 *sin6;
>
> sin6 = (struct sockaddr_in6 *)&ss;
> sin6->sin6_len = sizeof(*sin6);
> sin6->sin6_family = AF_INET6;
> sin6->sin6_port = htons(10000+(getpid()%10000));
> if (inet_pton(AF_INET6, "::1", &sin6->sin6_addr) <= 0)
> errx(1, "inet_pton()");
> } else
> usage();
>
> for (i = 0; i < NTHR; ++i) {
> pthread_t thr;
> int error;
>
> if ((error = pthread_create(&thr, NULL, thr_conn, &ss)))
> errc(1, error, "pthread_create()");
> }
>
> while(1) {
> int conns[NACCEPT];
>
> if ((s = socket(ss.ss_family, SOCK_STREAM, 0)) < 0)
> err(1, "socket()");
>
> if (ss.ss_family == AF_UNIX) {
> struct sockaddr_un *sun = (struct sockaddr_un *)&ss;
> unlink(sun->sun_path);
> } else {
> int opt = 1;
>
> if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR,
> &opt, sizeof(opt)) < 0)
> err(1, "setsockopt()");
> }
>
> if (bind(s, (struct sockaddr *)&ss, ss.ss_len) < 0)
> err(1, "bind()");
>
> if (listen(s, 10) < 0)
> err(1, "listen()");
>
> for(i=0; i < NACCEPT; ++i) {
> if ((conns[i] = accept(s, NULL, NULL)) < 0)
> err(1, "accept()");
> }
>
> close(s);
>
> for(i=0; i < NACCEPT; ++i) {
> close(conns[i]);
> }
> }
>
> return 0;
> }
>
> Index: sys/kern/uipc_socket.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.267
> diff -u -p -r1.267 uipc_socket.c
> --- sys/kern/uipc_socket.c 24 Oct 2021 07:02:47 -0000 1.267
> +++ sys/kern/uipc_socket.c 11 Nov 2021 08:56:17 -0000
> @@ -322,19 +322,9 @@ soclose(struct socket *so, int flags)
> s = solock(so);
> /* Revoke async IO early. There is a final revocation in sofree(). */
> sigio_free(&so->so_sigio);
> - if (so->so_options & SO_ACCEPTCONN) {
> - while ((so2 = TAILQ_FIRST(&so->so_q0)) != NULL) {
> - (void) soqremque(so2, 0);
> - (void) soabort(so2);
> - }
> - while ((so2 = TAILQ_FIRST(&so->so_q)) != NULL) {
> - (void) soqremque(so2, 1);
> - (void) soabort(so2);
> - }
> - }
> - if (so->so_pcb == NULL)
> - goto discard;
> if (so->so_state & SS_ISCONNECTED) {
> + if (so->so_pcb == NULL)
> + goto discard;
> if ((so->so_state & SS_ISDISCONNECTING) == 0) {
> error = sodisconnect(so);
> if (error)
> @@ -360,6 +350,16 @@ drop:
> error2 = (*so->so_proto->pr_detach)(so);
> if (error == 0)
> error = error2;
> + }
> + if (so->so_options & SO_ACCEPTCONN) {
> + while ((so2 = TAILQ_FIRST(&so->so_q0)) != NULL) {
> + (void) soqremque(so2, 0);
> + (void) soabort(so2);
> + }
> + while ((so2 = TAILQ_FIRST(&so->so_q)) != NULL) {
> + (void) soqremque(so2, 1);
> + (void) soabort(so2);
> + }
> }
> discard:
> if (so->so_state & SS_NOFDREF)
