On Thu, Nov 11, 2021 at 03:21:45PM +0100, Alexander Bluhm wrote:
> 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.
>
All fine with the current code. I wrote this to check that nothing
changes with the diff.
> 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)
