Date: Tue, 11 Sep 2001 00:32:18 -0400 (EDT) From: "Andrew R. Reiter" <arr@watson.org> To: freebsd-audit@freebsd.org Subject: select diffs Message-ID: <Pine.NEB.3.96L.1010910235947.45198A-200000@fledge.watson.org>
next in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] Hey, I am pondering starting on moving alot of the select() usages in src/libexec and src/usr.sbin to using dynamically allocated fd_set bit arrays in order to be extra cautious about fd usage. Therefore, I did a patch for rlogind so that people could review changes that would most likely occur to the sets of code that utilize select(2). I would appreciate comments... if you're wondering why select instead of perhaps moving to kqueue.... After speaking with Jonathan Lemon about whether it was worth moving from select() on all of these to a kqueue-ified system, I believe we/he concluded that probably none of these programs (libexec and usr.sbin) would really require kqueue-ification (except probably inetd), so I decided to move towards doing the fd_set's dyn. alloc'd rather than moving to a kq system. anyway, attached is a patch, but it can also be seen at: http://www.watson.org/~arr/fbsd-audit/libexec/rlogind/rlogin.c.diff This diff was made against current from 9/16/01 (evening pacific time) Andrew *-------------................................................. | Andrew R. Reiter | arr@fledge.watson.org | "It requires a very unusual mind | to undertake the analysis of the obvious" -- A.N. Whitehead [-- Attachment #2 --] --- rlogind.c.orig Mon Sep 10 22:32:28 2001 +++ rlogind.c Mon Sep 10 23:19:27 2001 @@ -87,6 +87,8 @@ #define ARGSTR "Dalnx" +#define fd_howmany(x, y) (((x) + ((y) - 1)) / (y)) + /* wrapper for KAME-special getnameinfo() */ #ifndef NI_WITHSCOPEID #define NI_WITHSCOPEID 0 @@ -383,48 +385,73 @@ fatal(f, "internal error (select mask too small)", 0); } for (;;) { - fd_set ibits, obits, ebits, *omask; + fd_set *ibits, *obits, *ebits, *omask; + + ibits = obits = ebits = omask = (fd_set *)NULL; + + ibits = (fd_set *)calloc(fd_howmany(nfd, NFDBITS), + sizeof(fd_mask)); + if (ibits == NULL) + fatal(f, "calloc", 1); + + obits = (fd_set *)calloc(fd_howmany(nfd, NFDBITS), + sizeof(fd_mask)); + if (obits == NULL) { + free(ibits); + fatal(f, "calloc", 1); + } - FD_ZERO(&ebits); - FD_ZERO(&ibits); - FD_ZERO(&obits); - omask = (fd_set *)NULL; + + ebits = (fd_set *)calloc(fd_howmany(nfd, NFDBITS), + sizeof(fd_mask)); + if (ebits == NULL) { + free(obits); + free(ibits); + fatal(f, "calloc", 1); + } + if (fcc) { - FD_SET(p, &obits); - omask = &obits; + FD_SET(p, obits); + omask = obits; } else - FD_SET(f, &ibits); + FD_SET(f, ibits); if (pcc >= 0) { if (pcc) { - FD_SET(f, &obits); - omask = &obits; + FD_SET(f, obits); + omask = obits; } else - FD_SET(p, &ibits); + FD_SET(p, ibits); } - FD_SET(p, &ebits); - if ((n = select(nfd, &ibits, omask, &ebits, 0)) < 0) { + FD_SET(p, ebits); + if ((n = select(nfd, ibits, omask, ebits, 0)) < 0) { + free(ibits); + free(obits); + free(ebits); if (errno == EINTR) continue; fatal(f, "select", 1); } if (n == 0) { /* shouldn't happen... */ + free(ibits); + free(obits); + free(ebits); sleep(5); continue; } #define pkcontrol(c) ((c)&(TIOCPKT_FLUSHWRITE|TIOCPKT_NOSTOP|TIOCPKT_DOSTOP)) - if (FD_ISSET(p, &ebits)) { + if (FD_ISSET(p, ebits)) { cc = read(p, &cntl, 1); if (cc == 1 && pkcontrol(cntl)) { cntl |= oobdata[0]; send(f, &cntl, 1, MSG_OOB); if (cntl & TIOCPKT_FLUSHWRITE) { pcc = 0; - FD_CLR(p, &ibits); + FD_CLR(p, ibits); } } } - if (FD_ISSET(f, &ibits)) { + if (FD_ISSET(f, ibits)) { #ifdef CRYPT if (doencrypt) fcc = des_enc_read(f, fibuf, sizeof(fibuf), @@ -439,7 +466,12 @@ int left, n; if (fcc <= 0) + { + free(obits); + free(ibits); + free(ebits); break; + } fbp = fibuf; top: @@ -456,11 +488,11 @@ goto top; /* n^2 */ } } - FD_SET(p, &obits); /* try write */ + FD_SET(p, obits); /* try write */ } } - if (FD_ISSET(p, &obits) && fcc > 0) { + if (FD_ISSET(p, obits) && fcc > 0) { cc = write(p, fbp, fcc); if (cc > 0) { fcc -= cc; @@ -468,19 +500,23 @@ } } - if (FD_ISSET(p, &ibits)) { + if (FD_ISSET(p, ibits)) { pcc = read(p, pibuf, sizeof (pibuf)); pbp = pibuf; if (pcc < 0 && errno == EWOULDBLOCK) pcc = 0; - else if (pcc <= 0) + else if (pcc <= 0) { + free(ibits); + free(obits); + free(ebits); break; + } else if (pibuf[0] == 0) { pbp++, pcc--; #ifdef CRYPT if (!doencrypt) #endif - FD_SET(f, &obits); /* try write */ + FD_SET(f, obits); /* try write */ } else { if (pkcontrol(pibuf[0])) { pibuf[0] |= oobdata[0]; @@ -489,7 +525,7 @@ pcc = 0; } } - if ((FD_ISSET(f, &obits)) && pcc > 0) { + if ((FD_ISSET(f, obits)) && pcc > 0) { #ifdef CRYPT if (doencrypt) cc = des_enc_write(f, pbp, pcc, @@ -503,8 +539,11 @@ * from p, but some old kernels balk at large * writes even when select returns true. */ - if (!FD_ISSET(p, &ibits)) + if (!FD_ISSET(p, ibits)) sleep(5); + free(obits); + free(ibits); + free(ebits); continue; } if (cc > 0) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1010910235947.45198A-200000>