From owner-freebsd-audit Sun Jun 17 3: 3:42 2001 Delivered-To: freebsd-audit@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id D153B37B405 for ; Sun, 17 Jun 2001 03:03:32 -0700 (PDT) (envelope-from bde@zeta.org.au) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id UAA14006; Sun, 17 Jun 2001 20:03:23 +1000 Date: Sun, 17 Jun 2001 20:01:36 +1000 (EST) From: Bruce Evans X-Sender: bde@besplex.bde.org To: Dima Dorfman Cc: audit@FreeBSD.ORG Subject: Re: Patch to fix `pstat -tn` In-Reply-To: <20010617021208.63AF83E28@bazooka.unixfreak.org> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Sat, 16 Jun 2001, Dima Dorfman wrote: > The attached patch fixes `pstat -tn` to do what the man page says it > should do, rather than just print '0'. I'm not sure when this was > broken (or if it ever worked at all), but the fix is relatively > simple. Please review. Lots was broken when pstat was converted to use the kern.ttys sysctl. pstat -nt still works for ttys not returned by this sysctl. OTOH, pstat -t is more broken for these ttys. Example of pstat -nt output: > LINE RAW CAN OUT IHIWT ILOWT OHWT LWT COL STATE SESS PGID DISC > 0 0 0 0 512 448 2052 256 59 OCc c128be00 2651 term > 0 0 0 0 512 448 2052 256 58 OCc c1283380 2203 term > 0 0 0 0 512 448 2052 256 0 OCc c11fe8c0 5934 term ^ The bug being discussed (all lines 0). > ... > 0 0 0 0 0 0 0 0 0 - 0 0 term > 0 0 0 0 512 448 2052 256 7 OCc c11d0440 378 term > 32 cy lines > LINE RAW CAN OUT IHIWT ILOWT OHWT LWT COL STATE SESS PGID DISC > 0 0 0 0 512 448 1296 256 0 - 0 0 term > 1 0 0 0 512 448 1296 256 0 - 0 0 term > 2 0 0 0 512 448 1296 256 0 - 0 0 term ^ The line number is printed correctly for the cy driver, since the cy driver doesn't support kern.ttys yet. The line number is apparently supposed to be just the ordinal number of the device, starting at 0 for each driver (see the code). This is not quite as described in the man page, but just as useful in this context. Note that the driver name is printed in a header line for non-broken cases, so the major number is less useful than in other contexts. Many more bugs are visible in the corresponding pstat -t output: > LINE RAW CAN OUT IHIWT ILOWT OHWT LWT COL STATE SESS PGID DISC > ttyp2 0 0 0 512 448 2052 256 59 OCc c128be00 2651 term > ttyp1 0 0 0 512 448 2052 256 58 OCc c1283380 2203 term > ttyp0 0 0 0 512 448 2052 256 0 OCc c11fe8c0 5985 term Bug: no header for the pty lines. Bug: pty lines are inversely ordered. > ttyv7 0 0 0 512 448 2052 256 7 OCc c11d0080 385 term > ttyv6 0 0 0 512 448 2052 256 7 OCc c11d0040 384 term > ttyv5 0 0 0 512 448 2052 256 7 OCc c11d0880 383 term > ttyv4 0 0 0 512 448 2052 256 7 OCc c11d0200 382 term > ttyv3 0 0 0 512 448 2052 256 7 OCc c11d02c0 381 term > ttyv2 0 0 0 512 448 2052 256 7 OCc c11d0840 380 term > ttyv1 0 0 0 512 448 2052 256 7 OCc c11d0340 379 term Bugs: similarly for the most sc lines. > ttyd0 0 0 0 11520 10080 4104 256 0 - 0 0 term > ttyd1 0 0 0 512 448 1296 256 0 - 0 0 term Bugs: similarly for the all sio lines. > consolectl 0 0 0 512 448 1296 256 0 OCc 0 0 term Bug: this sc line is not with the other sc lines. Bug: "consolectl" is too verbose for a tty name. Verbose break tables like this. Bug?: "consolectl" is not a normal tty (?), but we have allocated normal buffers for its i/o. > 0 0 0 0 0 0 0 0 0 - 0 0 term Bug: this line is too broken to even have a name. > ttyv0 0 0 0 512 448 2052 256 7 OCc c11d0440 378 term Bug: this sc line is more unsorted than the others. > 32 cy lines > LINE RAW CAN OUT IHIWT ILOWT OHWT LWT COL STATE SESS PGID DISC > #C:27:0x-105637 0 0 0 512 448 1296 256 0 - 0 0 term > #C:20:0x-105637 0 0 0 512 448 1296 256 0 - 0 0 term The names here are determined by devname(3), so bugs in devname(3) are inherited: Bug: devname() can't always find the device number in the database or in the kernel. (The above output is wihout devfs.) Bug: when devname() can't find the device number, it misprints minor numbers that aren't larger than 255 with format 0x%d. This is especially messy for negative minor numbers (see above), and the 0x prefix is nonsense. > ... > #C:169:0x-10563 0 0 0 512 448 1296 256 0 - 0 0 term > #C:162:0x-10563 0 0 0 512 448 1296 256 0 - 0 0 term > 16 0 0 0 0 0 0 0 0 - 0 0 term > ... > 31 0 0 0 0 0 0 0 0 - 0 0 term > Index: pstat.c > =================================================================== > RCS file: /home/ncvs/src/usr.sbin/pstat/pstat.c,v > retrieving revision 1.58 > diff -u -c -r1.58 pstat.c > *** pstat.c 2001/06/17 02:01:43 1.58 > --- pstat.c 2001/06/17 02:10:19 > *************** > *** 854,864 **** > int i, j; > pid_t pgid; > char *name, state[20]; > > if (usenumflag || tp->t_dev == 0 || > ! (name = devname(tp->t_dev, S_IFCHR)) == NULL) > ! (void)printf("%7d ", line); > ! else > (void)printf("%7s ", name); > (void)printf("%2d %3d ", tp->t_rawq.c_cc, tp->t_canq.c_cc); > (void)printf("%3d %5d %5d %4d %3d %7d ", tp->t_outq.c_cc, > --- 854,869 ---- > int i, j; > pid_t pgid; > char *name, state[20]; > + char *tb; > > if (usenumflag || tp->t_dev == 0 || > ! (name = devname(tp->t_dev, S_IFCHR)) == NULL) { > ! i = asprintf(&tb, "%d,%d", major(tp->t_dev), minor(tp->t_dev)); > ! if (i == -1) > ! err(1, "asprintf"); > ! (void)printf("%7s ", tb); > ! free(tb); > ! } else > (void)printf("%7s ", name); > (void)printf("%2d %3d ", tp->t_rawq.c_cc, tp->t_canq.c_cc); > (void)printf("%3d %5d %5d %4d %3d %7d ", tp->t_outq.c_cc, I think the correct fix is just to pass the correct value for `line' to ttyprt(). This is already done in the non-broken case (calls from ttytype()). This is not so easy for the calls from ttymode(), since the sysctl has lost the natural boundaries between the drivers. I don't like the patch for other reasons. Using asprintf() is overkill, and the format is inconsistent. All other places in pstat.c use " %2d,%-2d". This at least lines up the major/minor split point (the comma) for small major/minor numbers. It misprints negative minor numbers, but not as horribly as devname(). Large and negative minor numbers should be printed in hex format, something like ls(1) does it. Note that when the major and/or minor numbers are large, all the formats may take more than 7 characters and asprintf()'ing first doesn't help much (unless %7.7s format is used to truncate the names). Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message