svn commit: r194829 - in projects/libprocstat/sys: kern sys

[Robert Watson]

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