NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: kern/3019: a client can write to a read-only exported file system



Can you still reproduce PR 3019?

I can't reproduce it.

I set up an nfs server with a read/write file system /export/erlite3,
and a read-only export in /etc/exports, on 10.10.2.1:

server# grep -e -ro /etc/exports
/export/erlite3 -ro -network 10.10.2.0/24 -maproot=root:wheel
server# mount | grep erlite3
rpool/export/erlite3 on /export/erlite3 type zfs (noatime, NFS exported, local)

I mounted the file system read/write on the client 10.10.2.2:

client# mount
10.10.2.1:/export/erlite3 on / type nfs

And write attempts on the client fail with EROFS:

client# cd /tmp
client# touch x
touch: x: Read-only file system
client# cat >toto
-sh: cannot create toto: read-only file system


I reviewed all the NFS server RPC definitions in sys/nfs, and I
couldn't find one that fails to block writes on read-only exports with
EROFS.

Here's my notes from auditing these paths.  I also skimmed through the
same paths in the netbsd-1-1 branch, and I couldn't find any holes
there either, although I don't have any 1.1 or 1.2_BETA systems handy
to test.  So I'm struggling to find how this bug could have ever been
there, and if it was, how it was fixed in the time between when it was
filed and now.


* nfsrv_null

doesn't do anything

* nfsrv_getattr

read-only, VOP_GETATTR

* nfsrv_setattr

All paths go through a read-only check that fails with EROFS:

    359 	if (va.va_size == ((u_quad_t)((quad_t) -1))) {
    360 		if (rdonly || (vp->v_mount->mnt_flag & MNT_RDONLY)) {
    361 			error = EROFS;
    362 			goto out;
    363 		}
    364 	} else {
    365 		if (vp->v_type == VDIR) {
    366 			error = EISDIR;
    367 			goto out;
    368 		} else if ((error = nfsrv_access(vp, VWRITE, cred, rdonly,
    369 			lwp, 0)) != 0)
    370 			goto out;
    371 	}
    372 	error = VOP_SETATTR(vp, &va, cred);

https://nxr.netbsd.org/xref/src/sys/nfs/nfs_serv.c?r=1.184#359

* nfsrv_lookup

read-only, namei LOOKUP

* nfsrv3_access

read-only, VOP_GETATTR and VOP_ACCESS

* nfsrv_readlink

read-only, VOP_READLINK and VOP_GETATTR

* nfsrv_read

read-only, VOP_GETATTR, VOP_ACCESS, VOP_READ, uvm loan

* nfsrv_write

All paths to VOP_WRITE go through nfsrv_access check:

    927 	error = nfsrv_fhtovp(&nsfh, 1, &vp, cred, slp, nam,
    928 		 &rdonly, (nfsd->nd_flag & ND_KERBAUTH), false);
    929 	if (error) {
    930 		nfsm_reply(2 * NFSX_UNSIGNED);
    931 		nfsm_srvwcc_data(forat_ret, &forat, aftat_ret, &va);
    932 		return (0);
    933 	}
    934 	if (v3)
    935 		forat_ret = VOP_GETATTR(vp, &forat, cred);
    936 	if (vp->v_type != VREG) {
    937 		if (v3)
    938 			error = EINVAL;
    939 		else
    940 			error = (vp->v_type == VDIR) ? EISDIR : EACCES;
    941 	}
    942 	if (!error) {
    943 		nqsrv_getl(vp, ND_WRITE);
    944 		error = nfsrv_access(vp, VWRITE, cred, rdonly, lwp, 1);
    945 	}
    946 	if (error) {
    947 		vput(vp);
    948 		nfsm_reply(NFSX_WCCDATA(v3));
    949 		nfsm_srvwcc_data(forat_ret, &forat, aftat_ret, &va);
    950 		return (0);
    951 	}
...
    984 		error = VOP_WRITE(vp, uiop, ioflags, cred);

https://nxr.netbsd.org/xref/src/sys/nfs/nfs_serv.c?r=1.184#927

* nfsrv_create

This goes through namei with nameiop CREATE:


   1450 	error = nfs_namei(&nd, &nsfh, len, slp, nam, &md, &dpos,
   1451 		&dirp, (v3 ? &dirfor_ret : NULL), &dirfor,
   1452 		lwp, (nfsd->nd_flag & ND_KERBAUTH), false);

https://nxr.netbsd.org/xref/src/sys/nfs/nfs_serv.c?r=1.184#1450

    177 	error = nfsrv_fhtovp(nsfh, false, &dp, ndp->ni_cnd.cn_cred, slp,
    178 	    nam, &rdonly, kerbflag, pubflag);
    179 	if (error)
    180 		goto out;
    181 	if (dp->v_type != VDIR) {
    182 		vrele(dp);
    183 		error = ENOTDIR;
    184 		goto out;
    185 	}
    186 
    187 	if (rdonly)
    188 		cnp->cn_flags |= RDONLY;
...
    271 	error = lookup_for_nfsd(ndp, dp, neverfollow);

https://nxr.netbsd.org/xref/src/sys/nfs/nfs_srvsubs.c#177

lookup_for_nfsd goes through namei_tryemulroot:

   1978 	namei_init(&state, ndp);
   1979 	error = namei_tryemulroot(&state,
   1980 				  neverfollow, 1/*inhibitmagic*/, 1/*isnfsd*/);
   1981 	namei_cleanup(&state);
   1982 
   1983 	if (error) {
   1984 		/* make sure no stray refs leak out */
   1985 		KASSERT(ndp->ni_dvp == NULL);
   1986 		KASSERT(ndp->ni_vp == NULL);
   1987 	}
   1988 
   1989 	return error;

https://nxr.netbsd.org/xref/src/sys/kern/vfs_lookup.c?r=1.234#1978

namei_tryemulroot goes through namei_oneroot:

   1906 	error = namei_oneroot(state, neverfollow, inhibitmagic, isnfsd);

https://nxr.netbsd.org/xref/src/sys/kern/vfs_lookup.c?r=1.234#1906

namei_oneroot sets state->rdonly according to cnp->cn_flags & RDONLY:

   1507 	state->rdonly = cnp->cn_flags & RDONLY;

https://nxr.netbsd.org/xref/src/sys/kern/vfs_lookup.c?r=1.234#1507

It starts with lookup_fastforward:

   1533 		/*
   1534 		 * Parse out the first path name component that we need to
   1535 		 * to consider.  While doing this, attempt to use the name
   1536 		 * cache to fast-forward through as many "easy" to find
   1537 		 * components of the path as possible.
   1538 		 */
   1539 		error = lookup_fastforward(state, &searchdir, &foundobj);

https://nxr.netbsd.org/xref/src/sys/kern/vfs_lookup.c?r=1.234#1533

But that deliberately doesn't handle the last component:

   1306 		/*
   1307 		 * Can't deal with last component when modifying; this needs
   1308 		 * searchdir locked and VOP_LOOKUP() called (which can and
   1309 		 * does modify state, despite the name).  NB: this case means
   1310 		 * terminal is never set true when LOCKPARENT.
   1311 		 */
   1312 		if ((cnp->cn_flags & ISLASTCN) != 0) {
   1313 			if (cnp->cn_nameiop != LOOKUP ||
   1314 			    (cnp->cn_flags & LOCKPARENT) != 0) {
   1315 				error = EOPNOTSUPP;
   1316 				break;
   1317 			}
   1318 		}

https://nxr.netbsd.org/xref/src/sys/kern/vfs_lookup.c?r=1.234#1306

Back in namei_oneroot, it calls into lookup_once to handle the last
step:

   1541 		/*
   1542 		 * If we didn't get a good answer from the namecache, then
   1543 		 * go directly to the file system.
   1544 		 */
   1545 		if (error == EOPNOTSUPP) {
   1546 			error = lookup_once(state, searchdir, &searchdir,
   1547 			    &foundobj, &searchdir_locked);
   1548 		}

https://nxr.netbsd.org/xref/src/sys/kern/vfs_lookup.c?r=1.234#1541

And for the end of a path, lookup_once refuses with EROFS to create
anything if state->rdonly is set:

   1201 		/*
   1202 		 * If creating and at end of pathname, then can consider
   1203 		 * allowing file to be created.
   1204 		 */
   1205 		if (state->rdonly) {
   1206 			error = EROFS;
   1207 			goto done;
   1208 		}

https://nxr.netbsd.org/xref/src/sys/kern/vfs_lookup.c?r=1.234#1201

* nfsrv_mkdir

Blocked by namei CREATE like nfsrv_create.

* nfsrv_symlink

Blocked by namei CREATE like nfsrv_create.

* nfsrv_mknod

Blocked by namei CREATE like nfsrv_create.

* nfsrv_remove

Blocked by namei DELETE:

   1818 		/*
   1819 		 * Disallow directory write attempts on read-only lookups.
   1820 		 * Prefers EEXIST over EROFS for the CREATE case.
   1821 		 */
   1822 		if (state->rdonly &&
   1823 		    (cnp->cn_nameiop == DELETE || cnp->cn_nameiop == RENAME)) {
   1824 			if (searchdir) {
   1825 				if (searchdir_locked) {
   1826 					vput(searchdir);
   1827 					searchdir_locked = false;
   1828 				} else {
   1829 					vrele(searchdir);
   1830 				}
   1831 				searchdir = NULL;
   1832 			}
   1833 			vrele(foundobj);
   1834 			foundobj = NULL;
   1835 			ndp->ni_dvp = NULL;
   1836 			ndp->ni_vp = NULL;
   1837 			state->attempt_retry = 1;
   1838 			return EROFS;
   1839 		}

https://nxr.netbsd.org/xref/src/sys/kern/vfs_lookup.c?r=1.234#1818

* nfsrv_rmdir

Blocked by namei DELETE like nfsrv_remove.

* nfsrv_rename

Blocked by namei DELETE and RENAME like nfsrv_remove.

* nfsrv_link

Blocked by namei CREATE like nfsrv_create.

* nfsrv_readdir

read-only, VOP_GETATTR and VOP_READDIR

* nfsrv_readdirplus

read-only, VOP_GETATTR, VOP_READDIR, and VOP_VGET

* nfsrv_statfs

read-only, VFS_STATVFS and VOP_GETATTR

* nfsrv_fsinfo

read-only, VOP_GETATTR

* nfsrv_pathconf

read-only, VOP_PATHCONF

* nfsrv_commit

XXX VOP_FSYNC -- maybe this should be forbidden?

* nfsrv_noop

read-only, noop


Home | Main Index | Thread Index | Old Index