tech-userlevel archive

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

Sanitizing (canonicalising) the block device name in mount_ffs ??



I'm dual-posting this to tech-kern and tech-userlevel, as while it is
a userlevel issue, it could have kernel implications.   Please respect
the Reply-To and send replies only to tech-userlevel

You may have noticed that a recent change (mine) to the pathadj()
function (which converts an abritrary path name to its canonical form).
That function is not permitted to fail, but could.   Now instead of
failing, and returning (potential) nonsense, it exits if it cannot
do what it is required to do (usually it can).  In practice this
affects nothing real.

However, it affects some uses of rump - which sets up a "block device"
in a way that its name cannot be canonicalised.   It was relying upon
the way that pathadj() happens to work (based upon how realpath(3) works)
to make things function - pathadj() was issuing an error message, which
some rump using ATF tests were simply ignoring (deliberately).

Yesterday, I was trying to find a way to make this all work - unsuccessfully.

Today I am wondering why we need to bother?    That is, not why we bother
with rump, not even why rump has to make its magic etfs work the way it
does.   But why we need to canonicalise the block device name for mount.

I have run a test with that simply removed from mount_ffs (some of the
other mount_xxx's might eventually want to follow, if we change this)
and the ATF tests that are currently failing work again.

It is rare for any use of mount to be given a path for the block device
being mounted to be changed by canonicalising it, so simply omitting that
would not often make any difference at all.

Currently I am seeing 4 reasonable choices...

1) just omit the pathadj() of the block device name, and just use whatever
the user says to use, unchanged.   I doubt anything would really be affected
by this, but it does make a difference if some user were to use
	/./dev/../dev/wd0e
or	wd0e

where the latter is either a symlink to /dev/wd0e, or a copy of /dev/wd0e
or $PWD==/dev and it is simply a relative path.

Is anyone aware of anything which would break if we allowed such names to
be used - the dev_t that gets used for mounting is not going to change, not
even the vnode which is used - just the path name used to get to there ?

2) we could prohibit relative paths, or paths containing '.' or '..'
components - simply check the path and refuse the mount.

3) we could apply pathadj() (as it currently is) to the paths which choice 2
would prohibit (which won't affect the rump using ATF tests, which don't do
that).

4) we could change the pathadj() of the block device name to instead simply
call realpath(3), use the result if that succeeds (which is what happens now,
and in the past), but simply use the user's arg if it fails (which is what
will happen in the ATF test cases - using the original path is what is
needed there.)   Possibly issue a warning the way that pathadj() does.

I'd appreciate opinions, particularly if anyone knows of any reason that
any of these would be inappropriate as a solution.

kre



Home | Main Index | Thread Index | Old Index