Date: Wed, 24 Jun 2009 15:17:40 +0100 (BST) From: Robert Watson <rwatson@FreeBSD.org> To: Ulf Lilleengen <lulf@FreeBSD.org> Cc: svn-src-projects@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r194829 - in projects/libprocstat/sys: kern sys Message-ID: <alpine.BSF.2.00.0906241513070.13224@fledge.watson.org> In-Reply-To: <200906241207.n5OC7PZx013705@svn.freebsd.org> References: <200906241207.n5OC7PZx013705@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.00.0906241513070.13224>