Hi bluhm,

Thanks very much for your reply!

Yes, your patch covers all the corner cases, thanks!

On 10/2/2018 8:38 AM, Alexander Bluhm wrote:
> 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;
> 

-- 
Best Regards
Nan Xiao(肖楠)

Reply via email to