From owner-svn-src-projects@FreeBSD.ORG Wed Jun 24 14:17:40 2009 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D65B91065687; Wed, 24 Jun 2009 14:17:40 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id B1B3B8FC1B; Wed, 24 Jun 2009 14:17:40 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from fledge.watson.org (fledge.watson.org [65.122.17.41]) by cyrus.watson.org (Postfix) with ESMTPS id 6D2C746B3B; Wed, 24 Jun 2009 10:17:40 -0400 (EDT) Date: Wed, 24 Jun 2009 15:17:40 +0100 (BST) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: Ulf Lilleengen In-Reply-To: <200906241207.n5OC7PZx013705@svn.freebsd.org> Message-ID: References: <200906241207.n5OC7PZx013705@svn.freebsd.org> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-projects@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r194829 - in projects/libprocstat/sys: kern sys X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Jun 2009 14:17:41 -0000 On Wed, 24 Jun 2009, Ulf Lilleengen wrote: > @@ -3087,6 +3096,12 @@ sysctl_kern_proc_filedesc(SYSCTL_HANDLER > freepath = NULL; > fullpath = "-"; > FILEDESC_SUNLOCK(fdp); > + VOP_GETATTR(vp, &va, NULL); > + kif->kf_fsid = va.va_fsid; > + kif->kf_fileid = va.va_fileid; > + kif->kf_mode = MAKEIMODE(va.va_type, va.va_mode); > + kif->kf_size = va.va_size; > + kif->kf_rdev = va.va_rdev; You definitely want to handle VOP_GETATTR failing, both in terms of not dropping garbage into the structure, but perhaps also by indicating that the fields aren't filled out so userspace can display '-' instead of an uninitialized or improperly 0 size, for example. > #if defined(__amd64__) || defined(__i386__) > -#define KINFO_FILE_SIZE 1392 > +#define KINFO_FILE_SIZE 1412 (a) you don't want to change the ABI here, that's why we have spare fields, and (b) this can't be right, since you've added a field that varies in size between i386 and amd64 (see below). > #endif > > struct kinfo_file { > @@ -324,6 +324,11 @@ struct kinfo_file { > int kf_sock_protocol; /* Socket protocol. */ > struct sockaddr_storage kf_sa_local; /* Socket address. */ > struct sockaddr_storage kf_sa_peer; /* Peer address. */ > + long kf_fsid; /* Vnode filesystem id. */ > + long kf_fileid; /* Global file id. */ > + mode_t kf_mode; /* File mode. */ > + u_long kf_size; /* File size. */ > + dev_t kf_rdev; /* File device. */ u_long should never be exported from a kernel data structure to userspace, as it varies in size by ABI. For example, on an amd64 kernel, this will export kf_size as 64-bit, but 32-bit processes will expect it to be 32-bit. You probably off_t. > int _kf_ispare[16]; /* Space for more stuff. */ And you definitely want to be using spare fields so that you don't change the size of the structure. Robert N M Watson Computer Laboratory University of Cambridge