Date: Fri, 26 Jan 2007 01:21:41 -0600 From: "Scot Hetzel" <swhetzel@gmail.com> To: "Alexander Leidinger" <Alexander@leidinger.net> Cc: freebsd-emulation@freebsd.org Subject: Re: linuxolator: proc/filesystems and sysfs function implementations Message-ID: <790a9fff0701252321x2c7bea62od928766849e32c68@mail.gmail.com> In-Reply-To: <20070115105921.wbv2yrd4bkgokcko@webmail.leidinger.net> References: <790a9fff0701060012x40063f48pc842510b082df5a5@mail.gmail.com> <20070106130830.3c2e6d98@Magellan.Leidinger.net> <790a9fff0701132017g6c081567la4a759cea4618535@mail.gmail.com> <20070114105239.GA6955@stud.fit.vutbr.cz> <790a9fff0701141254s2c92b10ag6b042a019bc283c@mail.gmail.com> <20070115105921.wbv2yrd4bkgokcko@webmail.leidinger.net>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --] On 1/15/07, Alexander Leidinger <Alexander@leidinger.net> wrote: > Quoting Scot Hetzel <swhetzel@gmail.com> (from Sun, 14 Jan 2007 > 14:54:28 -0600): > > > On 1/14/07, Divacky Roman <xdivac02@stud.fit.vutbr.cz> wrote: > > >> + buf = (char *)(uintptr_t)args->arg2; > > : > >> + bsd_to_linux_fs(vfsp->vfc_name, &fs); > >> + len = strlen(fs.name) + 1; > >> + error = copyout(fs.name, buf, len); > >> > >> no need to check if it fits in the userspace buffer? also you > >> dont check return value > >> > > > > According to the linux man page, there isn't a way to check the size > > of buf, as we are only passed the value of the pointer in args->arg2. > > Ugh... there's not even an implicit size because of a fixed definition > of the target buffer in some header? That would be very bad design. > The kernel could overwrite userland data even if the kernel is not > malicious (load a new FS with a too long name and BOOOOM). > > > Should I replace buf with (char *)(uintptr_t)args->arg2 in the > > copyout, since it is only used there. > > > > http://www.die.net/doc/linux/man/man2/sysfs.2.html > > ---snip--- > BUGS > There is no libc or glibc support. There is no way to guess how large > buf should be. > ---snip--- > > That's bad. That's very very bad. I don't like this in the FreeBSD > kernel, I want an upper bound. Would you please try to find some > artificial upper bound? The MFSNAMELEN would be one candidate. A > better candidate would be a similar Linux value. > The patch is using MFSNAMELEN as the upper bound. > >> also... you strcpy string to another string, are you sure it always > >> fits? how do you > >> asure that? > >> > > > > name can't be larger than MFSNAMELEN, and sizeof(fs->name) == MFSNAMELEN. > > Please use strlcpy with the sizeof (defensive programming). > Changed strcpy to strlcpy. > > 16 is the value of MFSNAMELEN and used by the vfsconf struct to define > > the max size of vfc_name. I kept name (in the linux_file_system_type > > Is there a similar value in Linux? It would be better to use this if > it exists. > > > struct) the same size, so that we didn't have to worry about the size > > when using strcpy. > > When you find a value like MFSNAMELEN on Linux, please have a look at > linux_misc.c:linux_prctl(), specially the LINUX_PR_{SET|GET}_NAME case > and the comments in there. What you are doing here should be done > similar. > I wasn't able to locate a value like MFSNAMELEN. I looked at other implementations and the SGI version uses FSTYPESZ (same value as MFSNAMELEN), to limit the size of the buffer. The attached patch is using MFSNAMELEN to limit the size copied in (LINUX_GETFSIND) and copied out (LINUX_GETFSTYPE). LINUX_GETFSTYPE also assumes that the buffer we are copying too is >= MFSNAMELEN. I looked at the LTP sysfs02.c test, and in that file the buffer is set to 40 bytes. Attached is the updated patch, all of the code is now in linux_misc.{c,h}, linprocfs.c and linux_util.h (define for bsd_to_linux_fs function). Scot -- DISCLAIMER: No electrons were mamed while sending this message. Only slightly bruised. [-- Attachment #2 --] Index: compat/linprocfs/linprocfs.c =================================================================== RCS file: /home/ncvs/src/sys/compat/linprocfs/linprocfs.c,v retrieving revision 1.105 diff -u -r1.105 linprocfs.c --- compat/linprocfs/linprocfs.c 21 Jan 2007 13:18:52 -0000 1.105 +++ compat/linprocfs/linprocfs.c 26 Jan 2006 01:57:08 -0000 @@ -1041,6 +1133,51 @@ } /* + * Filler function for proc/filesystems + */ +static int +linprocfs_dofilesystems(PFS_FILL_ARGS) +{ + int i, fs_flags; + char fs_name[MFSNAMELEN]; + struct vfsconf *vfsp; + static char *linux_nodevfs[] = { + /* Filesystems included with FreeBSD */ + "coda", "devfs", "fdescfs", "mfs", "nfs", + "nfs4", "nullfs", "portalfs", "procfs", + "linprocfs", "smbfs", "linsysfs", "unionfs", + + /* Filesystems included with Linux */ + "9p", "afs", "autofs", "cifs", "configfs", + "debugfs", "devpts", "fuse", "hostfs", + "hppfs", "hugetlbfs", "jffs2", "ncpfs", + "nfsd", "openpromfs", "ramfs", "rootfs", + + NULL + }; + + TAILQ_FOREACH(vfsp, &vfsconf, vfc_list) { + + /* Does the filesystem require a dev entry? */ + fs_flags = 1; /* FS_REQUIRES_DEV */ + for (i = 0; linux_nodevfs[i] != NULL; i++) { + if (strcmp(linux_nodevfs[i], vfsp->vfc_name) == 0) { + fs_flags = 0; + break; + } + } + + strlcpy(fs_name, vfsp->vfc_name, sizeof(fs_name)); + bsd_to_linux_fs(fs_name); + + sbuf_printf(sb, "%s\t%s\n", \ + fs_flags ? "" : "nodev", fs_name); + } + + return (0); +} + +/* * Filler function for proc/cmdline */ static int @@ -1086,6 +1223,8 @@ NULL, NULL, PFS_RD); pfs_create_file(root, "devices", &linprocfs_dodevices, NULL, NULL, PFS_RD); + pfs_create_file(root, "filesystems", &linprocfs_dofilesystems, + NULL, NULL, PFS_RD); pfs_create_file(root, "loadavg", &linprocfs_doloadavg, NULL, NULL, PFS_RD); pfs_create_file(root, "meminfo", &linprocfs_domeminfo, Index: compat/linux/linux_misc.h =================================================================== RCS file: /home/ncvs/src/sys/compat/linux/linux_misc.h,v retrieving revision 1.2 diff -u -r1.2 linux_misc.h --- compat/linux/linux_misc.h 31 Dec 2006 11:56:16 -0000 1.2 +++ compat/linux/linux_misc.h 26 Jan 2006 02:12:30 -0000 @@ -42,4 +42,9 @@ #define LINUX_MAX_COMM_LEN 16 /* Maximum length of the process name. */ +/* defines for sysfs */ +#define LINUX_GETFSIND 1 /* Translate filesystem name to index */ +#define LINUX_GETFSTYP 2 /* Translate index to filesystem name */ +#define LINUX_GETNFSTYP 3 /* Return total number of filesystems */ + #endif /* _LINUX_MISC_H_ */ Index: compat/linux/linux_util.h =================================================================== RCS file: /home/ncvs/src/sys/compat/linux/linux_util.h,v retrieving revision 1.28 diff -u -r1.28 linux_util.h --- compat/linux/linux_util.h 27 Jun 2006 18:30:49 -0000 1.28 +++ compat/linux/linux_util.h 26 Jan 2006 02:01:34 -0000 @@ -101,4 +103,7 @@ char *linux_get_char_devices(void); void linux_free_get_char_devices(char *string); +/* used by sysfs and linprocfs_dofilesystems functions */ +void bsd_to_linux_fs(char *name); + #endif /* !_LINUX_UTIL_H_ */ Index: amd64/linux32/linux32_dummy.c =================================================================== RCS file: /home/ncvs/src/sys/amd64/linux32/linux32_dummy.c,v retrieving revision 1.7 diff -u -r1.7 linux32_dummy.c --- amd64/linux32/linux32_dummy.c 31 Dec 2006 13:16:00 -0000 1.7 +++ amd64/linux32/linux32_dummy.c 16 Jan 2007 02:42:03 -0000 @@ -50,6 +50,5 @@ DUMMY(get_kernel_syms); DUMMY(quotactl); DUMMY(bdflush); -DUMMY(sysfs); DUMMY(query_module); DUMMY(nfsservctl); Index: i386/linux/linux_dummy.c =================================================================== RCS file: /home/ncvs/src/sys/i386/linux/linux_dummy.c,v retrieving revision 1.45 diff -u -r1.45 linux_dummy.c --- i386/linux/linux_dummy.c 31 Dec 2006 13:16:00 -0000 1.45 +++ i386/linux/linux_dummy.c 16 Jan 2007 03:02:56 -0000 @@ -52,6 +52,5 @@ DUMMY(get_kernel_syms); DUMMY(quotactl); DUMMY(bdflush); -DUMMY(sysfs); DUMMY(vm86); DUMMY(query_module); Index: compat/linux/linux_misc.c =================================================================== RCS file: /home/ncvs/src/sys/compat/linux/linux_misc.c,v retrieving revision 1.205 diff -u -r1.205 linux_misc.c --- compat/linux/linux_misc.c 7 Jan 2007 19:30:19 -0000 1.205 +++ compat/linux/linux_misc.c 26 Jan 2006 04:56:52 -0000 @@ -1716,3 +1729,114 @@ { return (chroot(td, (struct chroot_args *)args)); } + +/* table used to derive linux filesystem names to/from freebsd filesystem names */ +static struct {const char *l_name; const char *b_name;} l2bfs_tbl[] = { + { "ext2", "ext2fs" }, + { "ext3", "ext2fs"}, + { "iso9660", "cd9660"}, + { "msdos", "msdosfs"}, + { "bsdprocfs", "procfs"}, + { "proc", "linprocfs"}, + { "sysfs", "linsysfs"}, + { "ufs", "ffs"}, + { "vfat", "msdosfs"}, + { NULL, NULL } +}; + +/* + * Translate bsd filesystem name to linux filesystem name + * used by linprocfs_dofilesystems and sysfs LINUX_GETFSTYP + */ +void +bsd_to_linux_fs(char *name) +{ + int i; + + for ( i = 0; l2bfs_tbl[i].b_name != NULL; i++) { + if (strcmp(l2bfs_tbl[i].b_name, name) == 0) { + strlcpy(name, l2bfs_tbl[i].l_name, sizeof(name)); + break; + } + } +} + +int +linux_sysfs(struct thread *td, struct linux_sysfs_args *args) +{ + int i, error=0; + unsigned int index = 0; + char fs_name[MFSNAMELEN]; + struct vfsconf *vfsp; + + switch (args->option) { + /* + * Translate the filesystem identifier string into a + * filesystem type index. + */ + case LINUX_GETFSIND: + /* Is args->arg1 valid, if not valid return EFAULT */ + error = copyinstr((void *)(register_t)args->arg1, fs_name, + MFSNAMELEN, NULL); + if (error) { + return (error); + } + + for ( i = 0; l2bfs_tbl[i].l_name != NULL; i++) { + if (strcmp(l2bfs_tbl[i].l_name, fs_name) == 0) { + strlcpy(fs_name, l2bfs_tbl[i].b_name, + sizeof(fs_name)); + break; + } + } + + TAILQ_FOREACH(vfsp, &vfsconf, vfc_list) + if (strcmp(vfsp->vfc_name, fs_name) == 0) { + td->td_retval[0] = index; + break; + } else + index++; + if (!vfsp) + return (EINVAL); + break; + + /* + * Translate the file-system type index into a + * null-terminated filesystem identifier string. + */ + case LINUX_GETFSTYP: + index = args->arg1; + + TAILQ_FOREACH(vfsp, &vfsconf, vfc_list) + if (index-- <= 0) + break; + if (!vfsp) + return (EINVAL); + + strlcpy(fs_name, vfsp->vfc_name, sizeof(fs_name)); + bsd_to_linux_fs(fs_name); + + /* + * We assume that the buffer pointed to by + * (void *)(register_t)args->arg2 is greater than MFSNAMELEN + */ + error = copyout(fs_name, (void *)(register_t)args->arg2, + MFSNAMELEN); + break; + + /* + * Return the total number of file system types + * currently present in the kernel. + */ + case LINUX_GETNFSTYP: + TAILQ_FOREACH(vfsp, &vfsconf, vfc_list) + index++; + td->td_retval[0] = index; + break; + + /* Invalid option passed */ + default: + error = EINVAL; + } + return (error); +}help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?790a9fff0701252321x2c7bea62od928766849e32c68>