Date: Wed, 18 May 2011 12:53:12 +0400 From: Sergey Kandaurov <pluknet@gmail.com> To: Rick Macklem <rmacklem@uoguelph.ca> Cc: freebsd-fs@freebsd.org Subject: Re: [old nfsclient] different nmount() args passed from mount vs. mount_nfs Message-ID: <BANLkTimvbZFcsdKPzEDWPot=XxMUU=-4-w@mail.gmail.com> In-Reply-To: <713535812.490291.1305679037413.JavaMail.root@erie.cs.uoguelph.ca> References: <BANLkTi=g=Y8=Ma=N1Ma_9-D0nP%2B673ay9Q@mail.gmail.com> <713535812.490291.1305679037413.JavaMail.root@erie.cs.uoguelph.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
On 18 May 2011 04:37, Rick Macklem <rmacklem@uoguelph.ca> wrote: >> 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. Thank you very much. After patch applied, at least old nfsclient works as expected. (I didn't test new nfsclient). ./mount -t oldnfs -o hostname=dev2.mail dev2.mail:/home/svn/freebsd/head /usr/src mount: dev2.mail:/home/svn/freebsd/head No server address: Invalid argument Can you commit the patch? > > 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. I see. My only concern was a crash. > (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. Yes, that's true. mount(8) doesn't specify a "hostname" option itself. -- wbr, pluknet
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BANLkTimvbZFcsdKPzEDWPot=XxMUU=-4-w>