From owner-freebsd-fs@FreeBSD.ORG Wed May 18 00:37:18 2011 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F2F8C106564A for ; Wed, 18 May 2011 00:37:18 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from esa-jnhn.mail.uoguelph.ca (esa-jnhn.mail.uoguelph.ca [131.104.91.44]) by mx1.freebsd.org (Postfix) with ESMTP id 8E80D8FC12 for ; Wed, 18 May 2011 00:37:18 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: ApwEAJET002DaFvO/2dsb2JhbACEWaI0iHCtWpB/hRKBBwSQEYcrh2Y X-IronPort-AV: E=Sophos;i="4.65,228,1304308800"; d="scan'208";a="125034784" Received: from erie.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.206]) by esa-jnhn-pri.mail.uoguelph.ca with ESMTP; 17 May 2011 20:37:17 -0400 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id 74164B3F28; Tue, 17 May 2011 20:37:17 -0400 (EDT) Date: Tue, 17 May 2011 20:37:17 -0400 (EDT) From: Rick Macklem To: Sergey Kandaurov Message-ID: <713535812.490291.1305679037413.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_Part_490290_2136409836.1305679037410" X-Originating-IP: [172.17.91.202] X-Mailer: Zimbra 6.0.10_GA_2692 (ZimbraWebClient - IE7 (Win)/6.0.10_GA_2692) Cc: freebsd-fs@freebsd.org Subject: Re: [old nfsclient] different nmount() args passed from mount vs. mount_nfs X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 May 2011 00:37:19 -0000 ------=_Part_490290_2136409836.1305679037410 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit > Hi. > > First, sorry for the long mail. I just tried to describe in full > details. > > When mounting nfs with some options, I found that /sbin/mount and > /sbin/mount_nfs pass options to nmount() differently, which results > in bad things (TM). I traced the options and here they are: > > From mount(8) -> mount_nfs(8): > "rw" -> "" > "addr" -> {something valid } > "fh" -> 5 > "sec" -> "sys" > "nfsv3" -> 0x0 => NFSMNT_NFSV3 > "hostname" -> "dev2.mail:/home/svn/freebsd/head" > "fstype" -> "oldnfs" > "fspath" -> "/usr/src" > "errmsg" -> "" > (nil) > > From pre-r221124 mount(8): > = "fstype" -> "oldnfs" > "hostname" -> "dev2.mail" > = "fspath" -> "/usr/src" > "from" -> "dev2.mail:/home/svn/freebsd/head" > = "errmsg" -> "" > (nil) > > Note, that pre-r221124 mount(8) knows nothing about oldnfs. > > 1. "hostname" option is passed differently from mount(8) and > mount_nfs(8). > When I force to mount oldnfs file system with mount(8) directly (to > not > bypass the nmount(2) call to mount_nfs(8)), I get this error: > ./mount -t oldnfs dev2.mail:/home/svn/freebsd/head /usr/src > mount: dev2.mail:/home/svn/freebsd/head Invalid hostname: Invalid > argument > > Hmm.. this may be because mount(8) passes value in $hostname:$path > format > (see the traces above). It might be due to different old nfsclient way > to parse > args, but I am not sure, I can be wrong. Anyway, it does not matter > now. > > The actual problem manifests when running the command with pre-r221124 > mount(8) binary. It knows nothing about "oldnfs" and (attention!) > calls nmount(2) > directly instead of bypassing the call to the mount_nfs(8) binary as > usually done, > and this is the place where the "unsanitized nmount(2) args" problem > is hidden. > [New mount knows about "oldnfs" and passes the call to mount_oldnfs(8) > that > prepares all the nmount(2) args to correctly hide the problem.] > > To prove it, that is how old and new mount(8) work differently: > 1) new mount(8) as of current > mount -d -t oldnfs dev2.mail:/home/svn/freebsd/head /usr/src > exec: mount_oldnfs dev2.mail:/home/svn/freebsd/head /usr/src > 2) old mount(8) as of pre-r221124 > ./mount -d -t oldnfs dev2.mail:/home/svn/freebsd/head /usr/src > mount -t oldnfs dev2.mail:/home/svn/freebsd/head /usr/src > > > Ok, back to the first paragraph: a different "hostname" mount option. > When I first faced with this, I tried to specify value for "hostname" > explicitly. Here it comes: > ./mount -t oldnfs -o hostname=dev2.mail > dev2.mail:/home/svn/freebsd/head /usr/src > [CABOOM!] > It just crashed. Do not do this :) > > Fatal trap 12: page fault while in kernel mode > cpuid = 0; apic id = 00 > fault virtual address = 0x1 > fault code = supervisor read data, page not present > instruction pointer = 0x20:0xffffffff805da299 > stack pointer = 0x28:0xffffff807bef6240 > frame pointer = 0x28:0xffffff807bef62a0 > code segment = base 0x0, limit 0xfffff, type 0x1b > = DPL 0, pres 1, long 1, def32 0, gran 1 > processor eflags = interrupt enabled, resume, IOPL = 0 > current process = 2541 (mount) > db> bt > Tracing pid 2541 tid 100076 td 0xfffffe0001ace460 > nfs_connect() at 0xffffffff805da299 = nfs_connect+0x79 > nfs_request() at 0xffffffff805da978 = nfs_request+0x398 > nfs_getattr() at 0xffffffff805e2a6c = nfs_getattr+0x2bc > VOP_GETATTR_APV() at 0xffffffff806f4283 = VOP_GETATTR_APV+0xd3 > mountnfs() at 0xffffffff805de739 = mountnfs+0x329 > nfs_mount() at 0xffffffff805dffc7 = nfs_mount+0xcf7 > vfs_donmount() at 0xffffffff804d46ff = vfs_donmount+0x82f > nmount() at 0xffffffff804d54f3 = nmount+0x63 > syscallenter() at 0xffffffff804861cb = syscallenter+0x1cb > syscall() at 0xffffffff806ae710 = syscall+0x60 > Xfast_syscall() at 0xffffffff8069922d = Xfast_syscall+0xdd > --- syscall (378, FreeBSD ELF64, nmount), rip = 0x800ab444c, rsp = > 0x7fffffffca48, rbp = 0x801009058 --- > > > As you might see from above nmount(2) args traces, mount(8) itself > doesn't > pass the "addr" option to the nmount(2) syscall while nfs_mount() > expects to > receive it, which is the problem. > Later deep in nmount(2) in /sys/nfsclient/nfs_krpc.c it tries to > dereference > addr value and page faults here in nfs_connect() : > > vers = NFS_VER3; > else if (nmp->nm_flag & NFSMNT_NFSV4) > vers = NFS_VER4; > XXX saddr is NULL, the next line will crash > if (saddr->sa_family == AF_INET) > if (nmp->nm_sotype == SOCK_DGRAM) > nconf = getnetconfigent("udp"); > > I think that nfsclient, probably in > sys/nfsclient/nfs_vfsops.c:mount_nfs(), > should handle a missing value for "addr" and/or "fh" mount options. > It doesn't check it currently: > Yes, at least for the case of "addr". I'm not sure if a zero length fh is considered ok for the old client or not. (It is valid for the new one.) I've attached a patch that does the check for the "addr=" option for both clients. You can test that if you'd like. It should avoid the crash. Since "oldnfs" didn't exist as a file system type pre-r21124, I don't think you can expect a pre-r211124 mount to be able to be done for it. (It will work for the default "nfs", it will just use the new NFS client.) > % static int > % nfs_mount(struct mount *mp) > % { > % struct nfs_args args = { > % [...] > % .addr = NULL, > % }; > % int error, ret, has_nfs_args_opt; > % int has_addr_opt, has_fh_opt, has_hostname_opt; > % struct sockaddr *nam; > > addr is initialized with NULL. num used later as a pointer to > args.addr value. > > % if ((mp->mnt_flag & (MNT_ROOTFS | MNT_UPDATE)) == MNT_ROOTFS) { > % error = nfs_mountroot(mp); > % goto out; > % } > > We do not try to mount root, this is not ours. > > % if (vfs_getopt(mp->mnt_optnew, "nfs_args", NULL, NULL) == 0) { > [...] > % has_nfs_args_opt = 1; > % } > > We do not use old mount(2) interface, not ours. > > % if (vfs_getopt(mp->mnt_optnew, "nfsv3", NULL, NULL) == 0) > % args.flags |= NFSMNT_NFSV3; > > mount(8) doesn't pass nfsv3 option, so NFSMNT_NFSV3 isn't set. > > % if (vfs_getopt(mp->mnt_optnew, "addr", (void **)&args.addr, > % &args.addrlen) == 0) { > % has_addr_opt = 1; > % if (args.addrlen > SOCK_MAXADDRLEN) { > % error = ENAMETOOLONG; > % goto out; > % } > % nam = malloc(args.addrlen, M_SONAME, > % M_WAITOK); > % bcopy(args.addr, nam, args.addrlen); > % nam->sa_len = args.addrlen; > % } > > mount(8) doesn't pass addr option, so args.addr isn't set, hence > struct sockaddr *nam is also NULL, has_addr_opt is 0. > > % if (vfs_getopt(mp->mnt_optnew, "hostname", (void **)&args.hostname, > % NULL) == 0) { > % has_hostname_opt = 1; > % } > % if (args.hostname == NULL) { > % vfs_mount_error(mp, "Invalid hostname"); > % error = EINVAL; > % goto out; > % } > > I don't know why I got here the error. I didn't analyze it deep > though. > "mount: dev2.mail:/home/svn/freebsd/head Invalid hostname: Invalid > argument" You'll get this if there is no hostname="xxx" argument specified, which I believe is correct. ------=_Part_490290_2136409836.1305679037410 Content-Type: text/x-patch; name=nfsmnt.patch Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename=nfsmnt.patch LS0tIG5mc2NsaWVudC9uZnNfdmZzb3BzLmMuc2F2CTIwMTEtMDUtMTcgMTk6NDg6MTUuMDAwMDAw MDAwIC0wNDAwCisrKyBuZnNjbGllbnQvbmZzX3Zmc29wcy5jCTIwMTEtMDUtMTcgMjA6MDA6NDYu MDAwMDAwMDAwIC0wNDAwCkBAIC0xMTQ5LDYgKzExNDksMTAgQEAgbmZzX21vdW50KHN0cnVjdCBt b3VudCAqbXApCiAJCQkJZ290byBvdXQ7CiAJCQl9CiAJCX0KKwl9IGVsc2UgaWYgKGhhc19hZGRy X29wdCA9PSAwKSB7CisJCXZmc19tb3VudF9lcnJvcihtcCwgIk5vIHNlcnZlciBhZGRyZXNzIik7 CisJCWVycm9yID0gRUlOVkFMOworCQlnb3RvIG91dDsKIAl9CiAJZXJyb3IgPSBtb3VudG5mcygm YXJncywgbXAsIG5hbSwgYXJncy5ob3N0bmFtZSwgJnZwLAogCSAgICBjdXJ0aHJlYWQtPnRkX3Vj cmVkLCBuZWduYW1ldGltZW8pOwotLS0gZnMvbmZzY2xpZW50L25mc19jbHZmc29wcy5jLnNhdgky MDExLTA1LTE3IDE4OjU2OjQ3LjAwMDAwMDAwMCAtMDQwMAorKysgZnMvbmZzY2xpZW50L25mc19j bHZmc29wcy5jCTIwMTEtMDUtMTcgMjA6MTA6NDcuMDAwMDAwMDAwIC0wNDAwCkBAIC0xMDc5LDE1 ICsxMDc5LDIxIEBAIG5mc19tb3VudChzdHJ1Y3QgbW91bnQgKm1wKQogCQlkaXJwYXRoWzBdID0g J1wwJzsKIAlkaXJsZW4gPSBzdHJsZW4oZGlycGF0aCk7CiAKLQlpZiAoaGFzX25mc19hcmdzX29w dCA9PSAwICYmIHZmc19nZXRvcHQobXAtPm1udF9vcHRuZXcsICJhZGRyIiwKLQkgICAgKHZvaWQg KiopJmFyZ3MuYWRkciwgJmFyZ3MuYWRkcmxlbikgPT0gMCkgewotCQlpZiAoYXJncy5hZGRybGVu ID4gU09DS19NQVhBRERSTEVOKSB7Ci0JCQllcnJvciA9IEVOQU1FVE9PTE9ORzsKKwlpZiAoaGFz X25mc19hcmdzX29wdCA9PSAwKSB7CisJCWlmICh2ZnNfZ2V0b3B0KG1wLT5tbnRfb3B0bmV3LCAi YWRkciIsCisJCSAgICAodm9pZCAqKikmYXJncy5hZGRyLCAmYXJncy5hZGRybGVuKSA9PSAwKSB7 CisJCQlpZiAoYXJncy5hZGRybGVuID4gU09DS19NQVhBRERSTEVOKSB7CisJCQkJZXJyb3IgPSBF TkFNRVRPT0xPTkc7CisJCQkJZ290byBvdXQ7CisJCQl9CisJCQluYW0gPSBtYWxsb2MoYXJncy5h ZGRybGVuLCBNX1NPTkFNRSwgTV9XQUlUT0spOworCQkJYmNvcHkoYXJncy5hZGRyLCBuYW0sIGFy Z3MuYWRkcmxlbik7CisJCQluYW0tPnNhX2xlbiA9IGFyZ3MuYWRkcmxlbjsKKwkJfSBlbHNlIHsK KwkJCXZmc19tb3VudF9lcnJvcihtcCwgIk5vIHNlcnZlciBhZGRyZXNzIik7CisJCQllcnJvciA9 IEVJTlZBTDsKIAkJCWdvdG8gb3V0OwogCQl9Ci0JCW5hbSA9IG1hbGxvYyhhcmdzLmFkZHJsZW4s IE1fU09OQU1FLCBNX1dBSVRPSyk7Ci0JCWJjb3B5KGFyZ3MuYWRkciwgbmFtLCBhcmdzLmFkZHJs ZW4pOwotCQluYW0tPnNhX2xlbiA9IGFyZ3MuYWRkcmxlbjsKIAl9CiAKIAlhcmdzLmZoID0gbmZo Owo= ------=_Part_490290_2136409836.1305679037410--