jail getfsstat patches.
Robert Watson
rwatson at freebsd.org
Sat Jun 26 08:06:01 PDT 2004
On Fri, 25 Jun 2004, Julian Elischer wrote:
> There are patches around to make 'df' and 'mount' show pretty much the
> exact right thing from a jail.
A couple of notes based on a quick skim and prior discussions:
(1) These patches get confused by unusual contortions of the BSD VFS, as
they rely on the strings for mounts, rather than actually determine if
the file system is visible. This is probably more acceptable than
determining if the file system is actually visible in the jail, but
should be kept in mind. For example, consider the renaming of
directories leading to mountpoints, and mounts legitimately created in
chroots (but not jails). This is a symptom of the general Problem
With Paths in BSD (see a post of mine a while back on arch@ on the
topic).
(2) This introduces the same dependency found in the mount system call
that the user process submits a mostly useful path to compare against.
Same Big BSD Path Problem, and we've seen some unfortunate bugs in the
user mount code where that assumption has problems -- for example, for
a while it wasn't possible to unmount a file system if its mountpoint
had been renamed since mount, as the mount command would scan the
mount table and reject it rather than unmount by path. This meant
that if a low level directory was renamed, or you wanted to unmount in
a chroot(), you were stuck. Caution advised.
(3) There's a lot of common code added to many functions. I'm sure that
can be abstracted. I.e.,
if (jail_statfs_restructed & jailed(td->td_ucred)) {
jail_statfs(td->td_ucred, mp);
mtx_lock(&mountlist_mtx);
nmp = TAILQ_NEXT(mp, mnt_list);
}
Likewise, the "is this path visible to this jail" is something that
needs to be abstracted, as it will help make it clear when it needs to
be called. I would advise adding comments on the order of:
/*
* We don't need to call jail_statfs_check() here because the
* process selected a file system by path, meaning that we already
* know it's in the process name space.
*/
(4) Should jail_statfs_restricted be per-jail?
(5) Is there any particular reason to mask the file system type on the
root file system? That's actually a somewhat useful piece of
information to have...
(6) The style in there is somewhat wrong, and should be fixed (a lot of
odd spacing, etc).
(7) If I might suggest, caching the suser result is OK, but I would avoid
combining the jail statfs restricted munging of the path information
in the same code block as the non-jail restriction on vsid exporting.
It makes the access control logic less clear.
(8) There might well be small races with forceable unmount here in the
presence of copyout's, etc. These may not be a big deal.
(9) The word "hack" appears in the comments. I wonder if we can find a
way to remove the cause of that word, and hence the comment. Or at
least, find a way to make it not a hack (or revise the
interpretation as a hack). For example, strlen() might be used.
Also, that use of strcpy() makes me nervous, since it's copying to and
from the same buffer, and if the string lengths come out
inconveniently, the two sections of the buffer may be overlapping.
(10) Could we assign constant names to the jail-statfs_restricted values?
When I see ">=4", I have to admit it isn't clear to me why at any
particular bit of the patch, that particular value was selected.
(11) If we ever allow mounting in jail, this code will instantly break.
Given that the person maintaining those web pages is a FreeBSD committer
who's been working actively on Jail stuff, I suggest his opinion as to
when to merge them is most relevant. :-)
Robert N M Watson FreeBSD Core Team, TrustedBSD Projects
robert at fledge.watson.org Principal Research Scientist, McAfee Research
>
> In both -current and 4.x
>
> I propose to commit these.
>
> http://garage.freebsd.pl/
> "jailfsstat - With this kernel module process in jail can only see file
> systems mounted inside."
>
> for 4.x
>
> and
>
> http://sources.zabbadoz.net/freebsd/jail.html
> for 5.x
>
> with possible small changes..
>
> e.g. the 4.x version would not be a module
> but would have a sysclt to turn it on
> (off by default)
>
> and the 5.x version may require osme small work too..
>
>
> Does anyone violently object to these?
>
> The fact that df or mount shows so much not only confuses the hell
> out of users, it makes scripts fail in odd ways.
> (and bugs the hell out of me too).
>
> julian
>
>
> _______________________________________________
> freebsd-current at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe at freebsd.org"
>
More information about the freebsd-current
mailing list