Re: [patch] Fix closing socket twice bug in netcat program
Hi bluhm, Very sorry for my remiss! Updated, thanks! Index: netcat.c === RCS file: /cvs/src/usr.bin/nc/netcat.c,v retrieving revision 1.192 diff -u -p -r1.192 netcat.c --- netcat.c10 Aug 2018 17:15:22 - 1.192 +++ netcat.c6 Sep 2018 10:10:07 - @@ -564,8 +564,11 @@ main(int argc, char *argv[]) } /* Allow only one connection at a time, but stay alive. */ for (;;) { - if (family != AF_UNIX) + if (family != AF_UNIX) { + if (s != -1) + close(s); s = local_listen(host, uport, hints); + } if (s < 0) err(1, NULL); if (uflag && kflag) { @@ -622,9 +625,7 @@ main(int argc, char *argv[]) } close(connfd); } - if (family != AF_UNIX) - close(s); - else if (uflag) { + if (family == AF_UNIX && uflag) { if (connect(s, NULL, 0) < 0) err(1, "connect"); } On 9/6/2018 8:53 PM, Alexander Bluhm wrote: > On Thu, Sep 06, 2018 at 06:27:15PM +0800, Nan Xiao wrote: >> @@ -564,8 +564,11 @@ main(int argc, char *argv[]) >> } >> /* Allow only one connection at a time, but stay alive. */ >> for (;;) { >> -if (family != AF_UNIX) >> +if (family != AF_UNIX) { >> +if (s == -1) > > This has to be (s != -1). > >> +close(s); >> s = local_listen(host, uport, hints); >> +} >> if (s < 0) >> err(1, NULL); >> if (uflag && kflag) { >> @@ -622,9 +625,7 @@ main(int argc, char *argv[]) >> } >> close(connfd); >> } >> -if (family != AF_UNIX) >> -close(s); >> -else if (uflag) { >> +if (family == AF_UNIX && uflag) { >> if (connect(s, NULL, 0) < 0) >> err(1, "connect"); >> } > > otherwise OK bluhm@ > -- Best Regards Nan Xiao(肖楠)
Re: [patch] Fix closing socket twice bug in netcat program
On Thu, Sep 06, 2018 at 06:27:15PM +0800, Nan Xiao wrote: > @@ -564,8 +564,11 @@ main(int argc, char *argv[]) > } > /* Allow only one connection at a time, but stay alive. */ > for (;;) { > - if (family != AF_UNIX) > + if (family != AF_UNIX) { > + if (s == -1) This has to be (s != -1). > + close(s); > s = local_listen(host, uport, hints); > + } > if (s < 0) > err(1, NULL); > if (uflag && kflag) { > @@ -622,9 +625,7 @@ main(int argc, char *argv[]) > } > close(connfd); > } > - if (family != AF_UNIX) > - close(s); > - else if (uflag) { > + if (family == AF_UNIX && uflag) { > if (connect(s, NULL, 0) < 0) > err(1, "connect"); > } otherwise OK bluhm@
Re: [patch] Fix closing socket twice bug in netcat program
Hi bluhm, Thanks for your reply! I think your method is better, and I update the patch: Index: netcat.c === RCS file: /cvs/src/usr.bin/nc/netcat.c,v retrieving revision 1.192 diff -u -p -r1.192 netcat.c --- netcat.c10 Aug 2018 17:15:22 - 1.192 +++ netcat.c6 Sep 2018 10:10:07 - @@ -564,8 +564,11 @@ main(int argc, char *argv[]) } /* Allow only one connection at a time, but stay alive. */ for (;;) { - if (family != AF_UNIX) + if (family != AF_UNIX) { + if (s == -1) + close(s); s = local_listen(host, uport, hints); + } if (s < 0) err(1, NULL); if (uflag && kflag) { @@ -622,9 +625,7 @@ main(int argc, char *argv[]) } close(connfd); } - if (family != AF_UNIX) - close(s); - else if (uflag) { + if (family == AF_UNIX && uflag) { if (connect(s, NULL, 0) < 0) err(1, "connect"); } Thanks! On 9/6/2018 5:07 AM, Alexander Bluhm wrote: > On Tue, Sep 04, 2018 at 01:01:38PM +0800, Nan Xiao wrote: >> Before netcat program exits, it will check whether s is -1, and close >> socket if s is not -1: >> >> if (s != -1) >> close(s); >> >> The following patch fixes the issue that netcat will close socket twice >> if it works as a server: > > I think it is a bug, but should be fixed differently. The netstat > code has a lot of if and else for all the use cases. Adding a s = > -1 at one place makes it even more confusing. > > In general main() does not reset s to -1, it isets it at the > beginning. Look at the client side. There we close at the beginning > of the loop: > > /* Cycle through portlist, connecting to each port. */ > for (s = -1, i = 0; portlist[i] != NULL; i++) { > if (s != -1) > close(s); > > So on the server side we should do the same, otherwise the code > will get more and more messy. Use the same concept everywhere. > I think this would be better: > > /* Allow only one connection at a time, but stay alive. */ > for (;;) { > if (family != AF_UNIX) { > if (s != -1) > close(s); > s = local_listen(host, uport, hints); > } > > bluhm > -- Best Regards Nan Xiao(肖楠)
Re: [patch] Fix closing socket twice bug in netcat program
On Tue, Sep 04, 2018 at 01:01:38PM +0800, Nan Xiao wrote: > Before netcat program exits, it will check whether s is -1, and close > socket if s is not -1: > > if (s != -1) > close(s); > > The following patch fixes the issue that netcat will close socket twice > if it works as a server: I think it is a bug, but should be fixed differently. The netstat code has a lot of if and else for all the use cases. Adding a s = -1 at one place makes it even more confusing. In general main() does not reset s to -1, it isets it at the beginning. Look at the client side. There we close at the beginning of the loop: /* Cycle through portlist, connecting to each port. */ for (s = -1, i = 0; portlist[i] != NULL; i++) { if (s != -1) close(s); So on the server side we should do the same, otherwise the code will get more and more messy. Use the same concept everywhere. I think this would be better: /* Allow only one connection at a time, but stay alive. */ for (;;) { if (family != AF_UNIX) { if (s != -1) close(s); s = local_listen(host, uport, hints); } bluhm
Re: [patch] Fix closing socket twice bug in netcat program
Ping tech@, Could anyone spare a minute to check this patch? I think it is indeed a bug. On 9/4/2018 1:01 PM, Nan Xiao wrote: > Hi tech@, > > Before netcat program exits, it will check whether s is -1, and close > socket if s is not -1: > > if (s != -1) > close(s); > > The following patch fixes the issue that netcat will close socket twice > if it works as a server: > > Index: netcat.c > === > RCS file: /cvs/src/usr.bin/nc/netcat.c,v > retrieving revision 1.192 > diff -u -p -r1.192 netcat.c > --- netcat.c 10 Aug 2018 17:15:22 - 1.192 > +++ netcat.c 4 Sep 2018 04:51:55 - > @@ -622,9 +622,10 @@ main(int argc, char *argv[]) > } > close(connfd); > } > - if (family != AF_UNIX) > + if (family != AF_UNIX) { > close(s); > - else if (uflag) { > + s = -1; > + } else if (uflag) { > if (connect(s, NULL, 0) < 0) > err(1, "connect"); > } > > Thanks! > -- Best Regards Nan Xiao(肖楠)
[patch] Fix closing socket twice bug in netcat program
Hi tech@, Before netcat program exits, it will check whether s is -1, and close socket if s is not -1: if (s != -1) close(s); The following patch fixes the issue that netcat will close socket twice if it works as a server: Index: netcat.c === RCS file: /cvs/src/usr.bin/nc/netcat.c,v retrieving revision 1.192 diff -u -p -r1.192 netcat.c --- netcat.c10 Aug 2018 17:15:22 - 1.192 +++ netcat.c4 Sep 2018 04:51:55 - @@ -622,9 +622,10 @@ main(int argc, char *argv[]) } close(connfd); } - if (family != AF_UNIX) + if (family != AF_UNIX) { close(s); - else if (uflag) { + s = -1; + } else if (uflag) { if (connect(s, NULL, 0) < 0) err(1, "connect"); } Thanks! -- Best Regards Nan Xiao