Bug #917

vnconfig -l support patch (Re: vn(4) RFC Misc.)

Added by c.turner over 6 years ago. Updated about 6 years ago.

Status:ClosedStart date:
Priority:LowDue date:
Assignee:-% Done:

0%

Category:-
Target version:-

Description

Per my recent email to kernel@, Attached is a little patch to get
vnconfig to list configured / available vn devices adapted from
& imho improved from OpenBSD.

Unless there are objections, I'll probably commit within the next couple
of days - I had a couple of questions / notes though particularly
on the VNIOCGET ioctl kernel code:

- I'm implicitly trusting the input, assuming that this is
checked on kernel copy-in time. Is this a correct assumption
(I didn't have time to track it down)

- The 'struct vn_user' doesn't store the vn file path, so I use the
vfs cache to re-lookup the vnode pointer backing the VN -
is this silly?

Also, I'm not sure if I needed to lock anything before using
the vfs cache, so there might be problems there.

- I've not yet run this under jails, which may mean a path disclosure
problem. Suggestions on fixing this if it is an issue welcome.

- I wasn't quite sure on how to do the swap size calculation for swap
backed vns in some cases & got bored at that point. Should work for
the normal case. Yea, lame excuse, I know.

Tested briefly on a vmware UP VM & vkernel so far.

Next up: Add a mount_vnd interface to vnconfig, after which it
might make sense to move this piece into /usr/src/sbin
instead of usr.sbin.

patch1-vnconfig-dash-ell (9.3 KB) c.turner, 01/14/2008 11:13 AM

History

#1 Updated by matthias over 6 years ago

Hi,

I really like your patch. Getting a list of vn's in use is really
helpful. The patch works fine here:

# vnconfig -l
vn0: covering /usr/scratch/test.iso on #B116:0x20055, inode 3221517

I would be happy if you could bring up some patches for encrypted vnode
disks :P I know, this discussion is old but I could not resist :)

Regards

Matthias

#2 Updated by dillon over 6 years ago

:Per my recent email to kernel@, Attached is a little patch to get
:vnconfig to list configured / available vn devices adapted from
:& imho improved from OpenBSD.
:
:Unless there are objections, I'll probably commit within the next couple
:of days - I had a couple of questions / notes though particularly
:on the VNIOCGET ioctl kernel code:

It looks great.

:- I'm implicitly trusting the input, assuming that this is
: checked on kernel copy-in time. Is this a correct assumption
: (I didn't have time to track it down)

If I remember right the kernel does the copyin/copyout for ioctl's
for you, so I believe the structure's integrity is good at the
point where you process it. You still have to bounds-check the
parameters of course (and it looks like you do).

Note that the length parameter encoded in the ioctl number
is limited to 8192 bytes (sys/ioccom.h). It can encode
the vn_user structure since PATH_MAX is only 1K, but be aware
of the limit.

:- The 'struct vn_user' doesn't store the vn file path, so I use the
: vfs cache to re-lookup the vnode pointer backing the VN -
: is this silly?
:
: Also, I'm not sure if I needed to lock anything before using
: the vfs cache, so there might be problems there.

I'd suggest just constructing the path by hand and assuming the
form "/dev/vn*". The namecache reference is not guarenteed
to exist... for example, the namecache can get flushed.

:- I've not yet run this under jails, which may mean a path disclosure
: problem. Suggestions on fixing this if it is an issue welcome.

It should theoretically work but if it doesn't it's a problem
with vn_fullpath(). curproc is always valid in the ioctl path so
you don't have to re-roll vn_fullpath(). Though, again, I suggest
just constructing the path manually and not using the namecache at all.

:- I wasn't quite sure on how to do the swap size calculation for swap
: backed vns in some cases & got bored at that point. Should work for
: the normal case. Yea, lame excuse, I know.
:
:Tested briefly on a vmware UP VM & vkernel so far.
:
:Next up: Add a mount_vnd interface to vnconfig, after which it
:might make sense to move this piece into /usr/src/sbin
:instead of usr.sbin.

/sbin is probably better then /usr/sbin, yah.

-Matt

#3 Updated by dillon over 6 years ago

:: Also, I'm not sure if I needed to lock anything before using
:: the vfs cache, so there might be problems there.
:
: I'd suggest just constructing the path by hand and assuming the
: form "/dev/vn*". The namecache reference is not guarenteed
: to exist... for example, the namecache can get flushed.

Oh, scratch that comment. You were trying to get the path to the
file VN was covering.

Use vn_fullpath() and pass 'curproc' as the process, though, instead
of cache_fullpath(), and note that it might fail (maybe have it print
'?' for the filename if it fails). If the process is non-NULL
vn_fullpath()/cache_fullpath() will truncate the path properly to
the process's chroot.

-Matt

#4 Updated by c.turner over 6 years ago

cheers - appreciate the review.

Taking it that redoing w/vn_fullpath is all that is needed on this patch
from your comments.. will commit w/edits in the next few days.

- Chris

Also available in: Atom PDF