On Sun, Sep 30, 2018 at 11:55:58AM +0800, Nan Xiao wrote:
> The following patch fixed the resource leak when netcat works as a TLS
> server. Sorry if I am wrong, thanks!

There is another tls leak on the client side after
                        if (s == -1)
                                continue;

To fix both I would do the tls_free() after close() consistently.

bluhm

Index: usr.bin/nc/netcat.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.194
diff -u -p -r1.194 netcat.c
--- usr.bin/nc/netcat.c 7 Sep 2018 09:55:29 -0000       1.194
+++ usr.bin/nc/netcat.c 2 Oct 2018 00:35:03 -0000
@@ -543,8 +543,6 @@ main(int argc, char *argv[])
                        err(1, "pledge");
        }
        if (lflag) {
-               struct tls *tls_cctx = NULL;
-               int connfd;
                ret = 0;
 
                if (family == AF_UNIX) {
@@ -603,6 +601,9 @@ main(int argc, char *argv[])
 
                                readwrite(s, NULL);
                        } else {
+                               struct tls *tls_cctx = NULL;
+                               int connfd;
+
                                len = sizeof(cliaddr);
                                connfd = accept4(s, (struct sockaddr *)&cliaddr,
                                    &len, SOCK_NONBLOCK);
@@ -618,12 +619,10 @@ main(int argc, char *argv[])
                                        readwrite(connfd, tls_cctx);
                                if (!usetls)
                                        readwrite(connfd, NULL);
-                               if (tls_cctx) {
+                               if (tls_cctx)
                                        timeout_tls(s, tls_cctx, tls_close);
-                                       tls_free(tls_cctx);
-                                       tls_cctx = NULL;
-                               }
                                close(connfd);
+                               tls_free(tls_cctx);
                        }
                        if (family == AF_UNIX && uflag) {
                                if (connect(s, NULL, 0) < 0)
@@ -657,6 +656,8 @@ main(int argc, char *argv[])
                for (s = -1, i = 0; portlist[i] != NULL; i++) {
                        if (s != -1)
                                close(s);
+                       tls_free(tls_ctx);
+                       tls_ctx = NULL;
 
                        if (usetls) {
                                if ((tls_ctx = tls_client()) == NULL)
@@ -707,18 +708,15 @@ main(int argc, char *argv[])
                                        tls_setup_client(tls_ctx, s, host);
                                if (!zflag)
                                        readwrite(s, tls_ctx);
-                               if (tls_ctx) {
+                               if (tls_ctx)
                                        timeout_tls(s, tls_ctx, tls_close);
-                                       tls_free(tls_ctx);
-                                       tls_ctx = NULL;
-                               }
                        }
                }
        }
 
        if (s != -1)
                close(s);
-
+       tls_free(tls_ctx);
        tls_config_free(tls_cfg);
 
        return ret;

Reply via email to