Skip site navigation (1)Skip section navigation (2)
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>