From owner-freebsd-emulation@FreeBSD.ORG Mon Jan 15 09:59:31 2007 Return-Path: X-Original-To: freebsd-emulation@freebsd.org Delivered-To: freebsd-emulation@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 5C6AC16A40F for ; Mon, 15 Jan 2007 09:59:31 +0000 (UTC) (envelope-from alexander@leidinger.net) Received: from redbull.bpaserver.net (redbullneu.bpaserver.net [213.198.78.217]) by mx1.freebsd.org (Postfix) with ESMTP id CC5A513C44C for ; Mon, 15 Jan 2007 09:59:30 +0000 (UTC) (envelope-from alexander@leidinger.net) Received: from outgoing.leidinger.net (p54A5D01F.dip.t-dialin.net [84.165.208.31]) by redbull.bpaserver.net (Postfix) with ESMTP id 980D82E216; Mon, 15 Jan 2007 11:06:34 +0100 (CET) Received: from webmail.leidinger.net (webmail.Leidinger.net [192.168.1.102]) by outgoing.leidinger.net (Postfix) with ESMTP id B8CDA5B497E; Mon, 15 Jan 2007 10:59:21 +0100 (CET) Received: (from www@localhost) by webmail.leidinger.net (8.13.8/8.13.8/Submit) id l0F9xLHp018715; Mon, 15 Jan 2007 10:59:21 +0100 (CET) (envelope-from Alexander@Leidinger.net) Received: from pslux.cec.eu.int (pslux.cec.eu.int [158.169.9.14]) by webmail.leidinger.net (Horde MIME library) with HTTP; Mon, 15 Jan 2007 10:59:21 +0100 Message-ID: <20070115105921.wbv2yrd4bkgokcko@webmail.leidinger.net> X-Priority: 3 (Normal) Date: Mon, 15 Jan 2007 10:59:21 +0100 From: Alexander Leidinger To: Scot Hetzel References: <790a9fff0701060012x40063f48pc842510b082df5a5@mail.gmail.com> <20070106130830.3c2e6d98@Magellan.Leidinger.net> <790a9fff0701132017g6c081567la4a759cea4618535@mail.gmail.com> <20070114105239.GA6955@stud.fit.vutbr.cz> <790a9fff0701141254s2c92b10ag6b042a019bc283c@mail.gmail.com> In-Reply-To: <790a9fff0701141254s2c92b10ag6b042a019bc283c@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; DelSp="Yes"; format="flowed" Content-Disposition: inline Content-Transfer-Encoding: quoted-printable User-Agent: Internet Messaging Program (IMP) H3 (4.1.3) / FreeBSD-7.0 X-BPAnet-MailScanner-Information: Please contact the ISP for more information X-BPAnet-MailScanner: Found to be clean X-BPAnet-MailScanner-SpamCheck: not spam, SpamAssassin (not cached, score=-14.71, required 6, BAYES_00 -15.00, DK_POLICY_SIGNSOME 0.00, FORGED_RCVD_HELO 0.14, TW_CP 0.08, TW_OC 0.08) X-BPAnet-MailScanner-From: alexander@leidinger.net X-Spam-Status: No Cc: freebsd-emulation@freebsd.org Subject: Re: linuxolator: proc/filesystems and sysfs function implementations X-BeenThere: freebsd-emulation@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Development of Emulators of other operating systems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Jan 2007 09:59:31 -0000 Quoting Scot Hetzel (from Sun, 14 Jan 2007 =20 14:54:28 -0600): > On 1/14/07, Divacky Roman wrote: >> + buf =3D (char *)(uintptr_t)args->arg2; > : >> + bsd_to_linux_fs(vfsp->vfc_name, &fs); >> + len =3D strlen(fs.name) + 1; >> + error =3D 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 =20 of the target buffer in some header? That would be very bad design. =20 The kernel could overwrite userland data even if the kernel is not =20 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 =20 buf should be. ---snip--- That's bad. That's very very bad. I don't like this in the FreeBSD =20 kernel, I want an upper bound. Would you please try to find some =20 artificial upper bound? The MFSNAMELEN would be one candidate. A =20 better candidate would be a similar Linux value. > I didn't check the return value, as error is going to be set to either > 0 or EFAULT by the copyout function. Since it's the end of the case, > it falls thru to the return statement at the bottom of the function. > >> + break; >> + >> + /* >> + * Return the total number of file system types =20 >> currently present >> + * in the kernel. >> + */ >> + case 3: >> + TAILQ_FOREACH(vfsp, &vfsconf, vfc_list) >> + index++; >> + td->td_retval[0] =3D index; >> >> does this make sense? shouldnt we return just the number of filesystems >> that we share with linux? >> > > There are only 7 filesystems that are freebsd specific (devfs, > fdescfs, mfs, nullfs, portalfs, procfs (bsd), unionfs). If we exclude > these filesystems, then we would need to ensure that we skip them in > the other two options. > > The way that it is currently emulated, is that all freebsd filesystems > (kldloaded filesystems) are available to the linuxolator. I suggest to keep the FreeBSD ones visible, we can revisit this =20 decision in case we find a case where whe shouldn't do this. >> also... you strcpy string to another string, are you sure it always =20 >> fits? how do you >> asure that? >> > > name can't be larger than MFSNAMELEN, and sizeof(fs->name) =3D=3D MFSNAMEL= EN. Please use strlcpy with the sizeof (defensive programming). >> 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 14 Jan 2007 02:08:42 -0000 >> @@ -101,4 +101,12 @@ >> char *linux_get_char_devices(void); >> void linux_free_get_char_devices(char *string); >> >> +struct linux_file_system_type { >> + char name[16]; >> + int fs_flags; >> +}; >> >> 16? why 16? please comment or something >> > 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 =20 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 =20 linux_misc.c:linux_prctl(), specially the LINUX_PR_{SET|GET}_NAME case =20 and the comments in there. What you are doing here should be done =20 similar. > I also wasn't sure if I should add: > > #include > > in linux_util.h so that MFSNAMELEN could be used to define the size of > name in the struct (it may also affect other files, that include > linux_util.h). I agree with Roman here. Do you intend to submit more patches to the linuxulator? If yes, do =20 you want to get write access to our perforce repository (that's not an =20 official CVS or developer account, but a step in this direction)? If =20 yes, what username do you prefer? Bye, Alexander. --=20 I'm still waiting for the advent of the computer science groupie. http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID =3D B0063FE7 http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID =3D 72077137