Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 3 Apr 2001 17:00:11 -0400 (EDT)
From:      Robert Watson <rwatson@FreeBSD.org>
To:        freebsd-arch@FreeBSD.org
Cc:        dillon@FreeBSD.org
Subject:   Eliminate crget() from nfs kernel code?
Message-ID:  <Pine.NEB.3.96L.1010403164943.7478F-100000@fledge.watson.org>

next in thread | raw e-mail | index | archive | help

While extending ucred in a rewrite of my mandatory access control
implementation on FreeBSD, I managed to panic the system:

kernel: type 12 trap, code=0
Stopped at      mac_free+0x15:  cmpxchgl        %ecx,0x20(%ebx)
db> trace
mac_free(0,c1055d00,c8a52ddc,c0234686,c1055d00) at mac_free+0x15
mac_free_subj(c1055d00,0,c8a52e2c,c02ed342,c1055d00) at mac_free_subj+0xf
crfree(c1055d00,c8a5a6c0,c0ff1200,c0ff124c,c89fba80) at crfree+0x82
nfs_statfs(c0ff1200,c0ff124c,c89fba80) at nfs_statfs+0x772
fstatfs(c89fba80,c8a52f80,81920c0,8156030,bfbea7ec) at fstatfs+0x44
syscall(2f,2f,2f,bfbea7ec,8156030) at syscall+0x3f5
syscall_with_err_pushed() at syscall_with_err_pushed+0x1b

What I discovered was that the nfs_statfs() implementation fakes up a
credential using crget() to use when invoking nfs_fsinfo(), and uses it
only for the lifetime of the nfs_statfs() call.  While I don't claim to
have a complete understanding of the NFS code, it appears that this is
done in lieu of using p->p_ucred, and that other related VFS calls are
passed a ucred as an argument.  This causes some problems for
implementations of extensions to ucred, as it introduces a new (and fairly
arbitrary) credential that must be created to satisfy the need to call
crget() here.  I was wondering if someone could expound on the rationale
for creating a new (and fairly poorly initialized) ucred in this scenario,
and perhaps comment also on two possible work-arounds:

  1) Use p->p_ucred.
  2) Introduce a ucred argument to vfs_statfs() to be used by
     file system implementations that require it.

Either would be in line with current precedent for VFS call construction: 
vfs_sync() has a ucred passed at invocation, the others generally assume
the use of p->p_ucred.  Using p->p_ucred would avoid changing the VFS
interface, but if it's really needed, I don't see any harm in adding a
ucred argument (although this will break binary compatibility in -CURRENT,
which is where I'd consider doing the change).  The effect of the current
code, btw, is to always make NFSPROC_STATFS()  calls using uid/gid of 0
(possibly mapped to nobody on the server), rather than the calling process
-- it's possible to imagine environments in which this is inappropriate,
especially with the introduction of MAC.  I notice that there's some
similar behavior in the ncp code.

Robert N M Watson             FreeBSD Core Team, TrustedBSD Project
robert@fledge.watson.org      NAI Labs, Safeport Network Services


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.NEB.3.96L.1010403164943.7478F-100000>