From msaitoh@spa.is.uec.ac.jp Tue Mar 9 10:21:01 1999 Return-Path: Received: from mailgate.spa.is.uec.ac.jp (ns.spa.is.uec.ac.jp [130.153.67.2]) by hub.freebsd.org (Postfix) with ESMTP id D47881504B for ; Tue, 9 Mar 1999 10:20:59 -0800 (PST) (envelope-from msaitoh@spa.is.uec.ac.jp) Received: from atami.spa.is.uec.ac.jp (atami.spa.is.uec.ac.jp [130.153.67.21]) by mailgate.spa.is.uec.ac.jp (8.8.8+2.7Wbeta7/3.6W+spa-2.0a) with ESMTP id DAA14908 for ; Wed, 10 Mar 1999 03:20:40 +0900 (JST) Received: by atami.spa.is.uec.ac.jp (8.8.8/3.6W+spa-null-2.0b) id DAA08352; Wed, 10 Mar 1999 03:20:40 +0900 (JST) Message-Id: <199903091820.DAA08352@atami.spa.is.uec.ac.jp> Date: Wed, 10 Mar 1999 03:20:40 +0900 (JST) From: SAITOH Masanobu Reply-To: msaitoh@spa.is.uec.ac.jp To: FreeBSD-gnats-submit@freebsd.org Subject: incorrect return value in kvm_read(3) and kvm_write(3) X-Send-Pr-Version: 3.2 >Number: 10511 >Category: bin >Synopsis: incorrect return value in kvm_read(3) and kvm_write(3) >Confidential: no >Severity: serious >Priority: high >Responsible: nectar >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Tue Mar 9 10:30:01 PST 1999 >Closed-Date: Fri Mar 31 07:05:13 PST 2000 >Last-Modified: Fri Mar 31 07:05:50 PST 2000 >Originator: SAITOH Masanobu >Release: all >Organization: University of Electro-Communications >Environment: -current >Description: manpage says: + RETURN VALUES + Upon success, the number of bytes actually transferred is returned. Oth- + erwise, -1 is returned. but, it returns 0 on error. And, some programs ckecks like: if (kvm_read(kd, addr, buf, len) < 0) This causes serious problems. >How-To-Repeat: This bug cause pidentd to infinite-loops. >Fix: return -1 on error. >Release-Note: >Audit-Trail: Responsible-Changed-From-To: freebsd-bugs->nectar Responsible-Changed-By: nectar Responsible-Changed-When: Thu Mar 30 12:45:40 PST 2000 Responsible-Changed-Why: Blame me. From: "Jacques A . Vidrine" To: freebsd-gnats-submit@freebsd.org Cc: Subject: Re: bin/10511 incorrect return value in kvm_read(3) and kvm_write(3) Date: Thu, 30 Mar 2000 14:43:38 -0600 This looks like a long-standing bug. The kvm_read/_write library functions return 0 on errors from at least as far back as our initial import of the BSD 4.4 Lite sources. The kvm_read/_write functions are part of no standard of which I know. IMHO that makes the source the ultimate authority. On the other hand, I understand that the kvm interface was leached from SunOS, and that platform is also documented to return -1 on errors. I can't check the source for these to see if that documentation is correct. It appears that most users of kvm_read/_write in our source tree check the return value of kvm_read to determine whether the number of bytes read is the same as the number requested, e.g. if (kvm_read(..., sizeof(*obj)) == sizeof(*obj)) This is safe whether kvm_read returns 0 or -1 as an error. There are some exceptions: usr.bin/fstat/fstat.c: < 0 usr.bin/ipcs/ipcs.c: == 0 usr.bin/nfsstat/nfsstat.c: < 0 usr.bin/vmstat/vmstat.c: < 0 Also I am unsure of how the return value of kvm_read is ultimately used by these: gnu/usr.bin/binutils/gdb/i386/kvm-fbsd.c gnu/usr.bin/binutils/gdb/alpha/kvm-fbsd.c Maybe someone (obrien?) can tell easily? I think the correct thing to do is to change kvm_read/_write to return (-1) on error, and fix the places in our source tree that were coded inconsitently with the documentation (apparently only ipcs.c). With that in mind, patches follow. I suppose this fixes potential bugs in fstat/nfsstat/vmstat. Index: lib/libkvm/kvm.c =================================================================== RCS file: /home/ncvs/src/lib/libkvm/kvm.c,v retrieving revision 1.13 diff -u -r1.13 kvm.c --- lib/libkvm/kvm.c 2000/03/27 00:33:44 1.13 +++ lib/libkvm/kvm.c 2000/03/30 20:16:38 @@ -375,12 +375,12 @@ errno = 0; if (lseek(kd->vmfd, (off_t)kva, 0) == -1 && errno != 0) { _kvm_err(kd, 0, "invalid address (%x)", kva); - return (0); + return (-1); } cc = read(kd->vmfd, buf, len); if (cc < 0) { _kvm_syserr(kd, 0, "kvm_read"); - return (0); + return (-1); } else if (cc < len) _kvm_err(kd, kd->program, "short read"); return (cc); @@ -391,7 +391,7 @@ cc = _kvm_kvatop(kd, kva, &pa); if (cc == 0) - return (0); + return (-1); if (cc > len) cc = len; errno = 0; @@ -437,19 +437,19 @@ errno = 0; if (lseek(kd->vmfd, (off_t)kva, 0) == -1 && errno != 0) { _kvm_err(kd, 0, "invalid address (%x)", kva); - return (0); + return (-1); } cc = write(kd->vmfd, buf, len); if (cc < 0) { _kvm_syserr(kd, 0, "kvm_write"); - return (0); + return (-1); } else if (cc < len) _kvm_err(kd, kd->program, "short write"); return (cc); } else { _kvm_err(kd, kd->program, "kvm_write not implemented for dead kernels"); - return (0); + return (-1); } /* NOTREACHED */ } Index: usr.bin/ipcs/ipcs.c =================================================================== RCS file: /home/ncvs/src/usr.bin/ipcs/ipcs.c,v retrieving revision 1.12 diff -u -r1.12 ipcs.c --- usr.bin/ipcs/ipcs.c 1999/12/29 05:05:32 1.12 +++ usr.bin/ipcs/ipcs.c 2000/03/30 20:21:24 @@ -216,7 +216,7 @@ } if ((display & (MSGINFO | MSGTOTAL)) && - kvm_read(kd, symbols[X_MSGINFO].n_value, &msginfo, sizeof(msginfo))) { + kvm_read(kd, symbols[X_MSGINFO].n_value, &msginfo, sizeof(msginfo))== sizeof(msginfo)) { if (display & MSGTOTAL) { printf("msginfo:\n"); -- Jacques Vidrine / n@nectar.com / nectar@FreeBSD.org State-Changed-From-To: open->closed State-Changed-By: nectar State-Changed-When: Fri Mar 31 07:05:13 PST 2000 State-Changed-Why: Corrected in revision 1.14 of src/lib/libkvm/kvm.c. Thanks for your report! >Unformatted: