Subject: struct netbsd32_statvfs and packed attribute
To: NetBSD Kernel <tech-kern@NetBSD.org>
From: Markus Mayer <mmayer@redback.com>
List: tech-kern
Date: 09/07/2007 16:36:21
This is a multi-part message in MIME format.
--------------080902090109000006000809
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit

Hi,

I just noticed an issue with netbsd32_statvfs and the getvfsstat() 
syscall. This is for a MIPS platform with 64 bit kernel and (some) 32 
bit userland applications. However, I am assuming it'll affect other 
platforms, as well.

There seems to be an incompatibility converting from the 64 bit 
structure in the kernel to the 32 bit structure in userland, as shown by 
my simple demo application.

Output of the test program shows: the first entry is copied out 
correctly, the next one is garbled, and all the other ones are empty 
altogether. This assumes the application is compiled as 32 bit binary 
and the kernel is 64 bit. The problem does *not* appear if kernel and 
userland are, both, 64 bit.

# ./mntinfo
mounted_fs=7
[0]     nfs     /       10.13.49.2:/usr/tftpboot/se/sticks/current
[1]                     15
[2]
[3]
[4]
[5]
[6]

I traced the problem to the attribute "packed" that is specified for 
struct netbsd32_statvfs. netbsd32_statvfs is 2228 bytes in the kernel. 
However, struct statvfs, which is not packed, is 2232 bytes in 32 bit 
userland.

Due to the size difference, incrementing the userspace "copyout" pointer 
(sfsp++) in the kernel (function netbsd32_getvfsstat()) does not do the 
right thing. The the pointer is not incremented "enough", so the kernel 
places data at addresses that userland does not expect, resulting in the 
above mess.

I removed the "packed" declaration and immediately got the desired results:

./mntinfo
mounted_fs=7
[0]     nfs     /       10.13.49.2:/usr/tftpboot/se/sticks/current
[1]     mfs     /dev    mfs:15
[2]     mfs     /tmp    mfs:831
[3]     mfs     /local  mfs:49
[4]     mfs     /p01    mfs:880
[5]     mfs     /p02    mfs:883
[6]     ffs     /md     /dev/wd2a

I checked "HEAD" in CVS and the "packed" attribute is still there for 
struct netbsd32_statvfs. I also noticed some structures there are only 
"packed" for x86_64 and others are "packed" for all architectures.

Wondering what the right "generic" thing to do is. For now, I changed it 
  as shown in the attached patch. It works for me, but I'm not sure if 
that's correct for all platforms, though.

I would appreciate it if somebody could have a look at this conversion 
process on other architectures and fix "HEAD" accordingly.

Thanks,
-Markus

-- 
Markus Mayer
Redback Networks Inc.
(604) 629-7251

--------------080902090109000006000809
Content-Type: text/plain;
 name="statvfs.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="statvfs.diff"

--- usr/src/sys/compat/netbsd32/netbsd32.h      14 Aug 2007 22:16:28 -0000
        1.1.6.3
+++ usr/src/sys/compat/netbsd32/netbsd32.h      7 Sep 2007 23:24:08 -0000
@@ -515,7 +515,11 @@
        char    f_fstypename[_VFS_NAMELEN]; /* fs type name */
        char    f_mntonname[_VFS_MNAMELEN];  /* directory on which mounted */
        char    f_mntfromname[_VFS_MNAMELEN];  /* mounted file system */
-} __attribute__((packed));
+}
+#ifdef __x86_64__
+__attribute__((packed))
+#endif
+;

 /* from <sys/timex.h> */
 typedef netbsd32_pointer_t netbsd32_ntptimevalp_t;

--------------080902090109000006000809
Content-Type: text/x-csrc;
 name="mntinfo.c"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="mntinfo.c"

#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/statvfs.h>

int main(int argc, char *argv[])
{
	long mntsize;
	int i, mounted_fs;
	struct statvfs *mntbuf;
	struct statvfs *buf;

	mounted_fs = getvfsstat(NULL, 0, ST_NOWAIT);
	printf("mounted_fs=%d\n", mounted_fs);
	buf = malloc(mounted_fs * sizeof(buf));
	if (buf == NULL) {
		fprintf(stderr, "%s: malloc failed -- %s\n", argv[0],
		    strerror(errno));
		return 1;
	}

	getvfsstat(buf, mounted_fs * sizeof(*buf), ST_NOWAIT);
	for (i = 0; i < mounted_fs; i++) {
		printf("[%d]\t%s\t\%s\t%s\n", i, buf[i].f_fstypename,
		    buf[i].f_mntonname, buf[i].f_mntfromname);
	}

	free(buf);
	return 0;
}

--------------080902090109000006000809--